Crate universe now renders separate BUILD files for each dependency #698
Conversation
There was a problem hiding this comment.
This generally looks good, but I'm concerned that we're dropping down to zero test coverage for BUILD file rendering. Can you add a test which asserts on the output of renderer, given a single hard-coded RenderablePackage?
This can be nicely reproducible, deterministic, and hard-coded, and I'm ok with deleting integration.rs after that (though we should add some unit tests of resolver at some point), but dropping to 0 coverage feels bad.
Co-authored-by: Daniel Wagner-Hall <dawagner@gmail.com>
|
I'll see what I can add. I initially tried adding the unit tests but felt that was a pretty decent rabbit hole and thought it would be better to do that in a dedicated PR. The changes can still be functionally tested using the examples so I wouldn't necessarily say there's zero coverage but definitely the state of testing right now is not ideal. |
|
I'm also a bit conflicted because a lot of this logic can be found in cargo-raze so getting deja-vu when looking to add tests. |
|
@illicitonion Is this a sufficient replacement of the tests? |
| # rules_rust crate_universe file format 1 | ||
| # config hash 598 | ||
|
|
There was a problem hiding this comment.
We should drop these lines from the output, they're only relevant for the lockfile which no longer uses them.
| @@ -13,6 +13,7 @@ use crate_universe_resolver::config::{Config, Override, Package}; | |||
| use crate_universe_resolver::NamedTempFile; | |||
|
|
|||
| #[test] | |||
| #[ignore] // TODO: This test fails because of the way `test` is implemented, see that method for details | |||
There was a problem hiding this comment.
Let's delete this whole file, no point keeping around a whole file of broken tests :)
* Remove now-obsolete integration tests - we have some unit test coverage now, and that coupled with our examples, should be sufficient. More unit tests will come over time. * Remove file format and config hash comment from generated output - the hash is now factored into the serialised JSON, and the file format is not currently relevant, though we may need to introduce a new equivalent in the future.
* Remove now-obsolete integration tests - we have some unit test coverage now, and that coupled with our examples, should be sufficient. More unit tests will come over time. * Remove file format and config hash comment from generated output - the hash is now factored into the serialised JSON, and the file format is not currently relevant, though we may need to introduce a new equivalent in the future.
* Remove now-obsolete integration tests - we have some unit test coverage now, and that coupled with our examples, should be sufficient. More unit tests will come over time. * Remove file format and config hash comment from generated output - the hash is now factored into the serialised JSON, and the file format is not currently relevant, though we may need to introduce a new equivalent in the future.
This change splits all the crate definitions into unique
BUILDfiles. The repository directory is updated fromto
In my experience working on
cargo-razethis makes testing and debugging much easier.