-
Notifications
You must be signed in to change notification settings - Fork 119
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
Add option for JSON formatted output #324
Conversation
This is gonna fail, but I want to see what the CI has to say.
Clippy was complaining about `dead_code` when building in non-test mode, so I'm going with this instead.
src/cmd/build.rs
Outdated
|
||
// we specified that debug symbols should be kept | ||
assert!(has_debug_symbols(&res.dest_wasm.unwrap())); | ||
|
||
Ok(()) | ||
}) | ||
} | ||
|
||
#[test] | ||
fn build_result_seralization_sanity_check() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh I couldn't come up with good tests to figure out whether or not this flag worked as expected. They basically ended up boiling down to whether or not BuildResult
had the right fields.
Open to suggestions though.
let args = crate::cmd::build::ExecuteArgs { | ||
manifest_path, | ||
build_artifact: BuildArtifacts::CodeOnly, | ||
..Default::default() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍!
|
||
let res = super::execute(args).expect("build failed"); | ||
|
||
assert!(res.serialize_json().is_ok()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert!(res.serialize_json().is_ok()); | |
// then | |
assert!(res.serialize_json().is_ok()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a
let expected output = /* Insert a multi-line string of the whole fixture */;
assert_eq!(serialized_json, expected_output);
You will have to replace some part of the absolute artifact paths in the serialized json (or use a regex).
Having a fixture for this will enable us to notice if the output format changes and communicate it in the changelog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add this test back in? It ran the whole execution pipeline with OutputType::Json
, which is not tested otherwise.
Co-authored-by: Michael Müller <michi@parity.io>
I've pulled the CI Docker image locally and all the tests pass. Could the issues be caching related? cc @TriplEight |
@HCastano if you run everything with
this, then it might be caching related. You can compare the executed tests by |
Just in case I've purged the caches and relaunched, now it's green, weird. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the execute
test from 730d14a#r687601916 back in, looks good otherwise!
Adds a new
--output-json
flag which will outputbuild
results in JSON instead of thenormal human readable format. This is useful for using
cargo-contract
in part of datapipelines.