Skip to content

enforce tests, rustfmt style, and non-style clippy checks #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
#
# Configuration for GitHub-based CI, based on the stock GitHub Rust config.
#
name: Rust

on:
push:
branches: [ main ]
pull_request:
branches: [ main ]

jobs:
check-style:
runs-on: ubuntu-18.04
steps:
# actions/checkout@v2
- uses: actions/checkout@28c7f3d2b5162b5ddd3dfd9a45aa55eaf396478b
- name: Report cargo version
run: cargo --version
- name: Report rustfmt version
run: cargo fmt -- --version
- name: Check style
run: cargo fmt -- --check

clippy-lint:
runs-on: ubuntu-18.04
steps:
# actions/checkout@v2
- uses: actions/checkout@28c7f3d2b5162b5ddd3dfd9a45aa55eaf396478b
- name: Report cargo version
run: cargo --version
- name: Report Clippy version
run: cargo clippy -- --version
- name: Run Clippy Lints
#
# Clippy overrides should go into src/lib.rs so that developers running
# `cargo clippy` see the same behavior as we see here in the GitHub
# Action. In some cases, the overrides also need to appear here because
# clippy does not always honor the crate-wide overrides. See
# rust-lang/rust-clippy#6610.
#
run: cargo clippy -- -A clippy::style -D warnings

windows-debug:
runs-on: windows-2019
steps:
# actions/checkout@v2
- uses: actions/checkout@28c7f3d2b5162b5ddd3dfd9a45aa55eaf396478b
- uses: actions-rs/toolchain@b223206e28798aa3c3668bdd6409258e6dc29172
with:
profile: minimal
toolchain: stable
override: true
- name: Report cargo version
run: cargo --version
- name: Build
run: cargo build --all-targets --verbose
- name: Run tests
run: cargo test --verbose -- --test-threads=1 cmd_run_recover

build-and-test:
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ ubuntu-18.04, windows-2019, macos-10.15 ]
steps:
# actions/checkout@v2
- uses: actions/checkout@28c7f3d2b5162b5ddd3dfd9a45aa55eaf396478b
- uses: actions-rs/toolchain@b223206e28798aa3c3668bdd6409258e6dc29172
with:
profile: minimal
toolchain: stable
override: true
- name: Report cargo version
run: cargo --version
- name: Build
run: cargo build --tests --verbose
- name: Run tests
run: cargo test --verbose
49 changes: 33 additions & 16 deletions examples/demo-provision.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use anyhow::Context;
use std::fs;
use std::io;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use steno::make_example_provision_saga;
Expand Down Expand Up @@ -58,6 +60,18 @@ fn make_saga_id() -> SagaId {
SagaId(Uuid::parse_str("049b2522-308d-442e-bc65-9bfaef863597").unwrap())
}

fn reader_for_log_input(
path: &Path,
) -> Result<Box<dyn io::Read>, anyhow::Error> {
if *path == PathBuf::from("-") {
Ok(Box::new(io::stdin()))
} else {
Ok(Box::new(fs::File::open(&path).with_context(|| {
format!("open recovery log \"{}\"", path.display())
})?))
}
}

/*
* "dot" subcommand
*/
Expand Down Expand Up @@ -103,9 +117,7 @@ struct PrintLogArgs {

async fn cmd_print_log(args: &PrintLogArgs) -> Result<(), anyhow::Error> {
let input_log_path = &args.input_log_path;
let file = fs::File::open(input_log_path).with_context(|| {
format!("open recovery log \"{}\"", input_log_path.display())
})?;
let file = reader_for_log_input(&input_log_path)?;
let sglog = SagaLog::load("unused", file).with_context(|| {
format!("load log \"{}\"", input_log_path.display())
})?;
Expand Down Expand Up @@ -147,9 +159,7 @@ async fn cmd_run(args: &RunArgs) -> Result<(), anyhow::Error> {
println!("recovering from log: {}", input_log_path.display());
}

let file = fs::File::open(&input_log_path).with_context(|| {
format!("open recovery log \"{}\"", input_log_path.display())
})?;
let file = reader_for_log_input(input_log_path)?;
let sglog = SagaLog::load(&args.creator, file).with_context(|| {
format!("load log \"{}\"", input_log_path.display())
})?;
Expand Down Expand Up @@ -204,17 +214,24 @@ async fn cmd_run(args: &RunArgs) -> Result<(), anyhow::Error> {
if let Some(output_log_path) = &args.dump_to {
let result = exec.result();
let log = result.saga_log;
let out = fs::OpenOptions::new()
.write(true)
.open(output_log_path)
.with_context(|| {
format!("open output log \"{}\"", output_log_path.display())
})?;
log.dump(out).with_context(|| {
format!("save output log \"{}\"", output_log_path.display())
})?;
let (mut stdout_holder, mut file_holder);
let (label, out): (String, &mut dyn io::Write) = if *output_log_path
== PathBuf::from("-")
{
stdout_holder = io::stdout();
(String::from("stdout"), &mut stdout_holder)
} else {
file_holder = fs::OpenOptions::new()
.write(true)
.open(output_log_path)
.with_context(|| {
format!("open output log \"{}\"", output_log_path.display())
})?;
(format!("\"{}\"", output_log_path.display()), &mut file_holder)
};
log.dump(out).with_context(|| format!("save output log {}", label))?;
if !args.quiet {
println!("dumped log to \"{}\"", output_log_path.display());
println!("dumped log to {}", label);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/saga_action_func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ pub struct ActionFunc<
* UndoFutType)>` is Sync and also satisfies our need to reference these
* type parameters in the struct's contents.
*/
#[allow(clippy::type_complexity)]
phantom: PhantomData<fn() -> (UserType, ActionFutType, UndoFutType)>,
}

Expand Down
6 changes: 3 additions & 3 deletions src/saga_exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,7 @@ impl<UserType: SagaType> SagaExecutor<UserType> {
let depth = max_parent_depth.unwrap() + 1;
max_depth_of_node.insert(node, depth);

nodes_at_depth.entry(depth).or_insert(Vec::new()).push(node);
nodes_at_depth.entry(depth).or_insert_with(Vec::new).push(node);
}

SagaExecStatus {
Expand Down Expand Up @@ -1685,7 +1685,7 @@ impl<UserType: SagaType> ActionContext<UserType> {
let item = self
.ancestor_tree
.get(name)
.expect(&format!("no ancestor called \"{}\"", name));
.unwrap_or_else(|| panic!("no ancestor called \"{}\"", name));
// TODO-cleanup double-asterisk seems ridiculous
serde_json::from_value((**item).clone())
.map_err(ActionError::new_deserialize)
Expand Down Expand Up @@ -1742,7 +1742,7 @@ impl<UserType: SagaType> ActionContext<UserType> {
.await
.child_sagas
.entry(self.node_id)
.or_insert(Vec::new())
.or_insert_with(Vec::new)
.push(Arc::clone(&e) as Arc<dyn SagaExecChild>);
Ok(e)
}
Expand Down
7 changes: 3 additions & 4 deletions src/saga_log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,15 +207,14 @@ impl SagaLog {
creator: self.creator.clone(),
};

let result = self
.record(event)
.expect(&format!("illegal event for node {}", node_id));
self.record(event)
.unwrap_or_else(|_| panic!("illegal event for node {}", node_id));

/*
* Although this implementation is synchronous, we want callers to
* behave as though it were async.
*/
async move { Ok(result) }
async move { Ok(()) }
}

fn record(&mut self, event: SagaNodeEvent) -> Result<(), SagaLogError> {
Expand Down
2 changes: 1 addition & 1 deletion src/saga_template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ impl SagaTemplateMetadata {
* Returns an object that can be used to print a graphiz-format
* representation of the underlying node graph.
*/
pub fn dot<'a>(&'a self) -> SagaTemplateDot<'a> {
pub fn dot(&self) -> SagaTemplateDot<'_> {
SagaTemplateDot(&self.graph)
}
}
Expand Down
12 changes: 6 additions & 6 deletions tests/test_smoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,12 @@ fn cmd_run_error() {
fn cmd_run_recover() {
/* Do a normal run and save the log so we can try recovering from it. */
let log = run_example("recover1", |exec| {
exec.arg("run").arg("--dump-to=/dev/stdout").arg("--quiet")
exec.arg("run").arg("--dump-to=-").arg("--quiet")
});

/* First, try recovery without having changed anything. */
let recovery_done = run_example("recover2", |exec| {
exec.arg("run").arg("--recover-from=/dev/stdin").stdin(log.as_str())
exec.arg("run").arg("--recover-from=-").stdin(log.as_str())
});
assert_contents("tests/test_smoke_run_recover_done.out", &recovery_done);

Expand All @@ -99,7 +99,7 @@ fn cmd_run_recover() {
"tests/test_smoke_run_recover_some.out",
&run_example("recover3", |exec| {
exec.arg("run")
.arg("--recover-from=/dev/stdin")
.arg("--recover-from=-")
.stdin(log_shortened.as_str())
}),
);
Expand All @@ -110,14 +110,14 @@ fn cmd_run_recover_unwind() {
/* Do a failed run and save the log so we can try recovering from it. */
let log = run_example("recover_fail1", |exec| {
exec.arg("run")
.arg("--dump-to=/dev/stdout")
.arg("--dump-to=-")
.arg("--quiet")
.arg("--inject-error=instance_boot")
});

/* First, try recovery without having changed anything. */
let recovery_done = run_example("recover_fail2", |exec| {
exec.arg("run").arg("--recover-from=/dev/stdin").stdin(log.as_str())
exec.arg("run").arg("--recover-from=-").stdin(log.as_str())
});
assert_contents(
"tests/test_smoke_run_recover_fail_done.out",
Expand All @@ -135,7 +135,7 @@ fn cmd_run_recover_unwind() {
"tests/test_smoke_run_recover_fail_some.out",
&run_example("recover_fail3", |exec| {
exec.arg("run")
.arg("--recover-from=/dev/stdin")
.arg("--recover-from=-")
.stdin(log_shortened.as_str())
}),
);
Expand Down
2 changes: 1 addition & 1 deletion tests/test_smoke_run_recover_done.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
recovering from log: /dev/stdin
recovering from log: -
recovered state
+ saga execution: sg-049b2522-308d-442e-bc65-9bfaef863597
+-- stage 1: done: InstanceCreate (produces "instance_id")
Expand Down
2 changes: 1 addition & 1 deletion tests/test_smoke_run_recover_fail_done.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
recovering from log: /dev/stdin
recovering from log: -
recovered state
+ saga execution: sg-049b2522-308d-442e-bc65-9bfaef863597
+-- stage 1: undone: InstanceCreate (produces "instance_id")
Expand Down
2 changes: 1 addition & 1 deletion tests/test_smoke_run_recover_fail_some.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
recovering from log: /dev/stdin
recovering from log: -
recovered state
+ saga execution: sg-049b2522-308d-442e-bc65-9bfaef863597
+-- stage 1: queued-undo: InstanceCreate (produces "instance_id")
Expand Down
2 changes: 1 addition & 1 deletion tests/test_smoke_run_recover_some.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
recovering from log: /dev/stdin
recovering from log: -
recovered state
+ saga execution: sg-049b2522-308d-442e-bc65-9bfaef863597
+-- stage 1: done: InstanceCreate (produces "instance_id")
Expand Down