jonhoo / inferno

A Rust port of FlameGraph
Other
1.64k stars 117 forks source link

Make it more obvious to a new contributor that the `flamegraph` git submodule needs to be initialised before running tests #290

Closed jacksonriley closed 1 year ago

jacksonriley commented 1 year ago

Without this change, if a contributor naively clones the repo and runs cargo test, they are met with:

...
failures:

---- collapse::dtrace::tests::test_collapse_multi_dtrace stdout ----
Error: Os { code: 2, kind: NotFound, message: "No such file or directory" }

---- collapse::dtrace::tests::test_collapse_multi_dtrace_simple stdout ----
Error: Os { code: 2, kind: NotFound, message: "No such file or directory" }

---- collapse::perf::tests::test_collapse_multi_perf stdout ----
Error: Os { code: 2, kind: NotFound, message: "No such file or directory" }

---- collapse::perf::tests::test_collapse_multi_perf_simple stdout ----
Error: Os { code: 2, kind: NotFound, message: "No such file or directory" }

failures:
    collapse::dtrace::tests::test_collapse_multi_dtrace
    collapse::dtrace::tests::test_collapse_multi_dtrace_simple
    collapse::perf::tests::test_collapse_multi_perf
    collapse::perf::tests::test_collapse_multi_perf_simple

With this change, they get some (hopefully) more obvious errors:

failures:

---- collapse::dtrace::tests::test_collapse_multi_dtrace stdout ----
thread 'collapse::dtrace::tests::test_collapse_multi_dtrace' panicked at 'Some tests require the flamegraph git submodule to be initialised, but it is not.
Initialise it with `git submodule update --init flamegraph`.', src/collapse/common.rs:790:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
...

I have two data points that this or something like it would be handy:

I haven't added a similar check for integration tests as unit tests run first so this error will be hit first anyway - but would be happy to do so if you wanted!

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 66.66% and project coverage change: -0.07 :warning:

Comparison is base (6326ced) 91.15% compared to head (53f23c0) 91.08%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #290 +/- ## ========================================== - Coverage 91.15% 91.08% -0.07% ========================================== Files 19 19 Lines 4238 4250 +12 ========================================== + Hits 3863 3871 +8 - Misses 375 379 +4 ``` | [Impacted Files](https://app.codecov.io/gh/jonhoo/inferno/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jon+Gjengset) | Coverage Δ | | |---|---|---| | [src/collapse/common.rs](https://app.codecov.io/gh/jonhoo/inferno/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jon+Gjengset#diff-c3JjL2NvbGxhcHNlL2NvbW1vbi5ycw==) | `66.45% <50.00%> (-0.29%)` | :arrow_down: | | [src/collapse/dtrace.rs](https://app.codecov.io/gh/jonhoo/inferno/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jon+Gjengset#diff-c3JjL2NvbGxhcHNlL2R0cmFjZS5ycw==) | `81.25% <100.00%> (+0.10%)` | :arrow_up: | | [src/collapse/perf.rs](https://app.codecov.io/gh/jonhoo/inferno/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jon+Gjengset#diff-c3JjL2NvbGxhcHNlL3BlcmYucnM=) | `89.23% <100.00%> (+0.03%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.