EmbarkStudios / crash-handling

Collection of crates to deal with crashes
Apache License 2.0
135 stars 13 forks source link

allow minidumper-test to operate as a binary for minidump-pipeline to manipulate #51

Closed Gankra closed 2 years ago

Gankra commented 2 years ago

Checklist

Description of Changes

This is kind of hacky, as I'm rapidly prototyping out the concept on this feature branch of minidump-pipeline.

With these two changesets, minidump-pipeline can now:

All intermediate artifacts (.exes, .syms, .dmps, ...) are stored on disk with a particular structure, and the final full-report.json gives you all of their paths.

Here is an example output on the CLI:

log.txt ``` all tests run! fpe ...passed! illegal ...passed! invalid-parameter ...passed! purecall ...passed! segv ...passed! stack-overflow ...passed! stack-overflow-c-thread ...failed to run! trap ...passed! 8 run, 7 passed, 1 failed, 0 skipped full report written to: C:\Users\ninte\dev\minidump-pipeline\runs\run\full-report.json Error: × some tests failed error: process didn't exit successfully: `target\debug\minidump-pipeline.exe --config pipeline-local.toml` (exit code: 1) ```

Here is a sample JSON report

full-report.json ``` { "stats": { "tests_run": 8, "tests_passed": 7, "tests_failed": 1, "tests_skipped": 0 }, "builds": { "dump_syms": { "installed": "C:\\Users\\ninte\\dev\\minidump-pipeline\\bin\\bin\\dump_syms.exe", "orig_bin_path": "C:\\Users\\ninte\\dev\\dump_syms\\target\\release\\dump_syms.exe" }, "minidump_stackwalk": { "installed": "C:\\Users\\ninte\\dev\\minidump-pipeline\\bin\\bin\\minidump-stackwalk.exe", "orig_bin_path": "C:\\Users\\ninte\\dev\\rust-minidump\\target\\release\\minidump-stackwalk.exe" }, "minidumper_test": { "installed": "C:\\Users\\ninte\\dev\\minidump-pipeline\\bin\\bin\\minidumper-test.exe", "orig_bin_path": "C:\\Users\\ninte\\dev\\crash-handling\\target\\release\\minidumper-test.exe" }, "crash_client": { "installed": "C:\\Users\\ninte\\dev\\minidump-pipeline\\bin\\bin\\crash-client.exe", "orig_bin_path": "C:\\Users\\ninte\\dev\\crash-handling\\target\\release\\crash-client.exe" } }, "syms": { "minidumper_test": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\syms\\minidumper_test.pdb\\DE71AD9D17F04DD69D0B7C4BA9E918102\\minidumper_test.sym", "crash_client": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\syms\\crash_client.pdb\\1937F6A032F943EF94EE525532B067B92\\crash_client.sym" }, "tests": { "fpe": { "status": "Passed", "dump": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\dumps\\fpe\\minidump.dmp", "reports": { "json_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\fpe\\report.json", "human_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\fpe\\report.human.txt", "logs": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\fpe\\report.log.txt" } }, "illegal": { "status": "Passed", "dump": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\dumps\\illegal\\minidump.dmp", "reports": { "json_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\illegal\\report.json", "human_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\illegal\\report.human.txt", "logs": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\illegal\\report.log.txt" } }, "invalid-parameter": { "status": "Passed", "dump": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\dumps\\invalid-parameter\\minidump.dmp", "reports": { "json_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\invalid-parameter\\report.json", "human_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\invalid-parameter\\report.human.txt", "logs": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\invalid-parameter\\report.log.txt" } }, "purecall": { "status": "Passed", "dump": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\dumps\\purecall\\minidump.dmp", "reports": { "json_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\purecall\\report.json", "human_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\purecall\\report.human.txt", "logs": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\purecall\\report.log.txt" } }, "segv": { "status": "Passed", "dump": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\dumps\\segv\\minidump.dmp", "reports": { "json_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\segv\\report.json", "human_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\segv\\report.human.txt", "logs": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\segv\\report.log.txt" } }, "stack-overflow": { "status": "Passed", "dump": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\dumps\\stack-overflow\\minidump.dmp", "reports": { "json_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\stack-overflow\\report.json", "human_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\stack-overflow\\report.human.txt", "logs": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\stack-overflow\\report.log.txt" } }, "stack-overflow-c-thread": { "status": "FailedRun", "dump": null, "reports": null }, "trap": { "status": "Passed", "dump": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\dumps\\trap\\minidump.dmp", "reports": { "json_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\trap\\report.json", "human_report": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\trap\\report.human.txt", "logs": "C:\\Users\\ninte\\dev\\minidump-pipeline\\runs\\run\\report\\trap\\report.log.txt" } } } } ``` Making minidumper-test into a binary makes minidump-pipeline have a very uniform and malleable architecture: it just wants to fetch and build a bunch of independent binaries, and stuff like `[patch]` or locks isn't a concern for customizing your toolchain. With this approach, individual projects can also ultimately use minidump-pipeline in CI as a kind of shared testsuite by doing something like: * cargo install minidump-pipeline * minidump-pipeline --config=config-that-uses-the-local-version-of-this-repo However, for this to be really useful we'll need/want: * A way to have more fine-grain tests than "produced output" * A way to declare test-expectations, keyed on various cfgs like "this test is known-busted on windows" or "skip this test on linux" Still, even without that stuff, this already massively automates getting a bunch of interesting data for inspecting "what happens on this platform/toolchain".
Gankra commented 2 years ago

oh, also I am increasingly inclined to move minidump-writer into this repo, as this will make all the "client" code live in one monorepo (except for like minidump-common, but whatever) which should make it a lot easier to atomically make changes to that part of the pipeline (similarly it would be nice for dump_syms to live with symbolic, but that dependency tree spiders through tons of things like gimli so less compelling).

Gankra commented 2 years ago

(currently messing around with factoring the crash-client binary into its own subcrate because cargo install --git only looks for Cargo.tomls...)

Gankra commented 2 years ago

🎊 I now have ci running on all 3 major platforms that runs through the whole pipeline and checks them against expectations (1 test marked as busted on macos and windows)!!!

https://github.com/Gankra/minidump-pipeline/runs/7512320396?check_suite_focus=true

Jake-Shadle commented 2 years ago

Oof, cargo really hates the double workspace. :(

Gankra commented 2 years ago

whoops yeah, I was zoomed in on the sub-directory and didn't notice the evil (and cargo test is already kinda broken on this repo for me so I'm not sure what's my fault or not for the other local failures...)

Gankra commented 2 years ago

ok this version of the hack, where it's both its own workspace member with a Cargo.toml and a [[bin]] of crash-client seems to make cargo test on mac and linux happy. windows is vaguely happy as long as I don't run the crash-handler tests (but those have always been broken for me, even on main).

Gankra commented 2 years ago

ah but build is understandably angry now:

warning: output filename collision.
The bin target `crash-client` in package `minidumper-test v0.1.0 (C:\Users\ninte\dev\crash-handling\minidumper-test)` has the same output filename as the bin target `crash-client` in package `crash-client v0.1.0 (C:\Users\ninte\dev\crash-handling\minidumper-test\crash-client)`.
Colliding filename is: C:\Users\ninte\dev\crash-handling\target\debug\deps\crash_client.exe
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.
Gankra commented 2 years ago

Ok this particular set of incantations seems to satisfy every possible way of trying to build/test all the various crates.

So now we basically are adding a \~fake Cargo.toml for crash-client that defines its own [workspace]. Unlike before, this doesn't cause problems for the parent workspace because crash-client isn't an explicit workspace-member. Then minidumper-test just reaches past the Cargo.toml and builds main.rs as if everything was exactly like it was before.

But now the jank heuristics of cargo install --git can find this \~fake Cargo.toml and pick up how to build the binary in isolation.

It's possible this wouldn't be necessary if I taught minidump-pipeline that minidumper-test and crash-client are two binaries for the price of one (that is, that installing minidumper-test should in principle also install crash-client, because it's a [[bin]] of the package), but it's kind of nice to treat all the binaries uniformly. hrm.

Gankra commented 2 years ago

oh totally, I expected even more pushback lol

Jake-Shadle commented 2 years ago

Nah, those crates are just for testing so they can change more/be weirder, don't even necessarily need to publish them at all.