Rust Based Regression Tester #188
Conversation
jamesmunns
changed the title
|
Seems good to me, in principle. Replacing the bash script with a Rust program also makes it easier to run the test suite on Windows.
The Travis builders have 1 VCPU, though, so this would only make local builds faster. I think while doing this we could also move from testing a whole vendor in a single CI build to more evenly split all the devices across several CI builds. I'd like to hear what others think. |
|
re: travis and 1VCPU, I am hoping to later offer my GitLab CI builders, which have 16 VCPUs, so once that is implemented, it should help with those builds. Either way, it definitely could help with local builds, where it is typical to have 2-8+ threads available. I am really open to other possible options for CI options, including specifying specific targets or vendors, maybe tagging targets (e.g. "these SVDs have clusters, and should be tested when anything regarding clusters is changed"). |
ci/svd2rust-regress/src/main.rs
Outdated
| f.write_all(svd.as_bytes()).unwrap(); | ||
| } | ||
|
|
||
| let output = Command::new("/home/james/personal/svd2rust/target/release/svd2rust") |
|
Seems good. Obviously there needs to be more done but the idea is better imo than having it scripted |
jamesmunns
force-pushed
the
rust-ci
branch
2 times, most recently
from
3dd2ba6 to
8ef24fb
Compare
|
Hey @japaric @Emilgardis @ryankurte, I believe I have a good first effort for this tester. I have not attempted to remove the existing CI, I plan to tackle that in another PR. Feel free to check out the rendered README.md for more information, and let me know if something is not clear. This should allow users on any supported rust platform (Linux/OSX/Windows) to run regression tests easily. |
jamesmunns
changed the title
| let any_fails = AtomicBool::new(false); | ||
|
|
||
| // TODO: It would be more efficient to reuse directories, so we don't | ||
| // have to rebuild all the deps crates |
There was a problem hiding this comment.
You can place all the tests in a workplace to fix this.
There was a problem hiding this comment.
@Emilgardis this would prevent all the tests from running in parallel (as-is), as they would be competing for a single workspace lock.
I also tried putting them in a single workspace and ran a cargo build --all at the workspace level, and that seemed to be possible theoretically, but that caused some spurious failures in my testing (perhaps resource exhaustion?). If you feel strongly about it, I can reinvestigate, though I would probably think we could address this as an improvement in the future.
|
I may also add one more commit that adds a per-test retry threshold to prevent spurious failures (at an individual crate level) from causing a whole-ci failure. I am open to thoughts on this :) |
|
Also Also here is an example of a full regression test build using my CI servers: https://gitlab.onevariable.com/james/svd2rust/-/jobs/72 |
|
It seems that some of my spurious failures (mentioned above) are due to out of memory conditions, as some crates take upward of 3.2G of memory to compile (particularly the freescale chips). I don't know how to work around this, other than document that the testing will require significant amounts of CPU and memory, and hope that people have swap[file] configured. |
|
Hey all, I'd like to get this merged if possible. Is there anything additional needed before merging this? Should it be mentioned in the main README? |
|
I think a small mention wouldn't hurt. |
|
LGTM.
Yeah, it should be mention that svd2rust-regress can be used to run the test suite faster on a local machine. |
|
@japaric @Emilgardis added a mention in the main README. Okay to merge? |
In order to make running CI tests easier (and faster), I'd like to show you a bit of a hacky PoC I am working on.
This would replace the Bash based CI scripts with a small Rust program. This is largely to take advantage of
rayon, since compilation of crates during CI is generally single-CPU bound when compiling a single largelib.rsgenerated bysvd2rust.This is not ready to merge, but I am mostly looking for feedback from @japaric and @Emilgardis whether this is something you would consider merging.