-
Notifications
You must be signed in to change notification settings - Fork 70
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
Fix ci breaking due to new dependencies and rustc changes (backport to master) #163
Conversation
@@ -27,7 +27,7 @@ pyo3 = { version = "0.20", optional = true, default-features = false } | |||
futures = { version = "0.3", default-features = false } | |||
rustversion = "1.0" | |||
thiserror = "1.0" | |||
trybuild = { version = "1.0.19", features = ["diff"] } | |||
trybuild = { version = "=1.0.83", features = ["diff"] } |
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.
Needed for #150 to pass
@@ -124,6 +124,8 @@ fn test_panic_backwards_compatibility() { | |||
.args(["run", "--example", "theme_test_helper"]) | |||
.arg("--no-default-features") | |||
.args(&features) | |||
.env("RUSTFLAGS", "-Awarnings") | |||
.env("CARGO_FUTURE_INCOMPAT_REPORT_FREQUENCY", "never") |
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.
This removes cargo printing an extra line on windows warning about a future incompatability in nom.
The cargo output is included in the panic test case as it invokes cargo run and looks at the output.
Since the report is colored with ansi-codes, it gets included in the test 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.
Finished dev [unoptimized + debuginfo] target(s) in 0.03s
Running `target/debug/examples/theme_test_helper`
�[31mThe application panicked (crashed).�[0m
Message: �[36m<non string panic payload>�[0m
Location: �[35mexamples/theme_test_helper.rs�[0m:�[35m37�[0m
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ SPANTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
0: �[91mtheme_test_helper�[0m�[91m::�[0m�[91mget_error�[0m with �[96m�[3mmsg�[0m�[2m=�[0m"test"�[0m
at �[35mexamples/theme_test_helper.rs�[0m:�[35m34�[0m
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
�[96m ⋮ 6 frames hidden ⋮ �[0m
7: �[32mstd::panic::panic_any�[0m�[90m::hd76a7f826307234c�[0m
at �[35m/rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panic.rs�[0m:�[35m57�[0m
8: �[91mtheme_test_helper::main�[0m�[90m::h767d3fd6c45048c8�[0m
at �[35m/home/jlusby/git/yaahc/color-eyre/examples/theme_test_helper.rs�[0m:�[35m37�[0m
9: �[32mcore::ops::function::FnOnce::call_once�[0m�[90m::hc5a1cd4127189dad�[0m
at �[35m/rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/ops/function.rs�[0m:�[35m227�[0m
�[96m ⋮ 15 frames hidden ⋮ �[0m
Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
Run with RUST_BACKTRACE=full to include source snippets.
This is what we are checking against, note how it included the cargo output as well as the running output.
This has been an issue in the past wrt existing warnings, which was fixed by fixing the warnings.
This warning will fix itself in nom and we can not currently do anything about the warning
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.
This seems completely satisfactory.
Applies the same fixes that was applied to
color-eyre-0.6
to master, namely the new update totrybuild
that breaks our MSRV requirement, requiring us to pin the version, as the dependency only bumped the patch version for the updated MRSV.It also fixes the theme test breaking on windows due to new before-main machinery on windows affecting the backtrace. This is fixed by filtering out the functions before and including main(due to no take_until iter adapter in std), but still includes the panicking function call.
There are currently three branches that need/needed this fix