TheZoq2
commented
1 year ago Updated to also pass tests now. I hope I did that part correctly.
Should I also add a test to make sure that these style options are applied?
Closed TheZoq2 closed 1 year ago
TheZoq2
commented
1 year ago Updated to also pass tests now. I hope I did that part correctly.
Should I also add a test to make sure that these style options are applied?
TheZoq2
commented
1 year ago After adding the binary flag in the latest commit, this is now failing on my local machine with
---- collapse::dtrace::tests::test_collapse_multi_dtrace_simple stdout ----
thread 'collapse::dtrace::tests::test_collapse_multi_dtrace_simple' panicked at 'assertion failed: `(left == right)`
left: `1`,
right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/test/src/lib.rs:184:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- collapse::dtrace::tests::test_collapse_multi_dtrace stdout ----
thread 'collapse::dtrace::tests::test_collapse_multi_dtrace' panicked at 'assertion failed: `(left == right)`
left: `1`,
right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/test/src/lib.rs:184:5
---- collapse::perf::tests::test_collapse_multi_perf stdout ----
thread 'collapse::perf::tests::test_collapse_multi_perf' panicked at 'assertion failed: `(left == right)`
left: `1`,
right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/test/src/lib.rs:184:5
---- collapse::perf::tests::test_collapse_multi_perf_simple stdout ----
thread 'collapse::perf::tests::test_collapse_multi_perf_simple' panicked at 'assertion failed: `(left == right)`
left: `1`,
right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/test/src/lib.rs:184:5It feels like those things shouldn't be related, right?
jonhoo
commented
1 year ago It feels like those things shouldn't be related, right?
I agree. Our tests don't generally fail spuriously so it's a bit concerning, but unlikely to be related to your change :thinking:
Also, looks like there's a conflict with main
TheZoq2
commented
1 year ago Turns out the reason those tests were failing was that I forgot git update --init :slightly_smiling_face:
The UiColor struct was not intentional, I initially started out trying to use it instead of just using Option<Color>. Then I accidentally replaced the wrong thing with the LSP before getting rid of its use. Should be better now
jonhoo
commented
1 year ago Oh, and would you mind also updating the CHANGELOG file?
TheZoq2
commented
1 year ago Updated with all three changes
codecov[bot]
commented
1 year ago Base: 90.32% // Head: 90.28% // Decreases project coverage by -0.03% :warning:
Coverage data is based on head (
a835462) compared to base (eeea9f5). Patch coverage: 96.29% of modified lines in pull request are covered.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
jonhoo
commented
1 year ago Published as 0.11.13 :tada:
I wanted to embed the flamegraphs in a html document with a dark theme. While setting the background works fine, I couldn't find a way to the text color of the "UI elements", so this is an attempt to add that option.
The search button is quite faint here, maybe I should do something about that
Should I also add these options to the CLI part? For my personal use, the library part is enough, but I suppose others may want it via CLI.
I'm very unfamiliar with this code base, so let me know if this is not the right way to do it. Also, feel free to disregard if this is out of scope