0xPolygonZero / zk_evm

Apache License 2.0
69 stars 21 forks source link

CI should run workspace tests, not per crate #342

Closed atanmarko closed 7 hours ago

Nashtare commented 1 week ago

Any particular reason why? I find it clearer (especially when working on cross-crates functionality) to see where the tests are failing, than a single and global Run tests job. As long as we don't have 20 jobs running concurrently per PR, I'd say that's fine. Eventually combining the 3 scripts jobs would make more sense to me (though this should probably wait until #288 is addressed).

atanmarko commented 1 week ago

@Nashtare I would say there is a lot of duplicated boilerplate code (DRY principle). Lot of duplicated wasted actions (download source code, download and install Rust for every test). Lot of manual tweaking of tests paths. Easily forgotten tests when we add/refactor crates. Tests setup is growing more and more complex and harder to maintain. You may skip some common workspace rules and configuration (and tests should follow common build settings). Etc... :) In general when the test fails in the workspace test call, it would show test name. I guess it is debatable if that is more clear than separate per crate test job running in CI test list (we are already at ~15 jobs there).

atanmarko commented 8 hours ago

@Nashtare Any decision on this one? Should we keep separate tests or unify them in one workspace run?

Nashtare commented 8 hours ago

No strong opinion on this. I tend to lean towards individual crate test jobs, but if you and others feel preferable to have a whole workspace job for testing, then I say let's go for it.

@muursh do you have any preference?

muursh commented 7 hours ago

I err towards individual crate tests too tbh.

atanmarko commented 7 hours ago

@muursh @Nashtare Ok, then we keep things as they are.

CC: @0xaatif @BGluth

BGluth commented 3 hours ago

Yeah I actually have a slight preference for a single cargo test action for the reasons that Marko mentioned above, but also this is just a slight preference.