-
Notifications
You must be signed in to change notification settings - Fork 127
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 test for condition_script_runner_args expansion #1157
base: master
Are you sure you want to change the base?
Add test for condition_script_runner_args expansion #1157
Conversation
let step = Step { | ||
name: "test".to_string(), | ||
config: Task { | ||
install_crate: Some(InstallCrate::CrateInfo(InstallCrateInfo { |
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.
install_crate
here doesnt do what i intended here since its run after the condition script is validated. I should just remove it.
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.
@sagiegurari is there a better way to ensure that rust-script 0.35.0 is available for this test?
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.
you can create multiple tasks with dependencies.
so one task will do install and another task depending on the first, will do the condition.
@sagiegurari It looks like the test is failing only on MSRV rust on ubuntu. I (very hackily) forced some additional debug output and it looks like rust-script fails to be installed w/ msrv rust. I see 2 issues with this:
Do you agree that these changes make sense to make? |
@wmmc88 yes both changes (lock and validate) makes sense. do you want to PR that? |
Yes. I'll update this pr with the changes |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1157 +/- ##
==========================================
- Coverage 92.82% 92.46% -0.37%
==========================================
Files 119 119
Lines 26302 26344 +42
==========================================
- Hits 24416 24360 -56
- Misses 1886 1984 +98 ☔ View full report in Codecov by Sentry. |
@@ -15,7 +15,7 @@ jobs: | |||
strategy: | |||
fail-fast: false | |||
matrix: | |||
rust: [stable, beta, nightly, 1.73.0] | |||
rust: [stable, beta, nightly, 1.74.0] |
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.
rust-script's msrv is now 1.74:
error: failed to compile `rust-script v0.35.0`, intermediate artifacts can be found at `/tmp/cargo-install75pEMP`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.
Caused by:
package `clap_lex v0.7.2` cannot be built because it requires rustc 1.74 or newer, while the currently active rustc version is 1.73.0
@sagiegurari The changes are implemented. MSRV was also bumped one version to 1.74 to accomodate for rust-script bumping its MSRV |
Added an additional test for functionality that #1132 added. This was made possible by a fix included in rust-script v 0.35.0 that fixes fornwall/rust-script#135