dev(sozo): improve test coverage and other few changes #1462
Conversation
lambda-0x
force-pushed
the
sozo-improvements
branch
from
2c3d3c2
to
f748e29
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1462 +/- ##
==========================================
+ Coverage 68.03% 69.40% +1.37%
==========================================
Files 228 228
Lines 22074 22138 +64
==========================================
+ Hits 15017 15364 +347
+ Misses 7057 6774 -283 ☔ View full report in Codecov by Sentry. |
crates/sozo/src/commands/build.rs
Outdated
|
|
||
| use super::BuildArgs; | ||
|
|
||
| // this adds considerable time to test suite, should this be enabled by default? |
There was a problem hiding this comment.
i think this should be done as part of the dojo-test-utils build.rs already?
There was a problem hiding this comment.
build.rs has it but it doesn't generate bindings. since build is done this shouldn't be expensive to do assuming cairo compiler does incremental builds
There was a problem hiding this comment.
As the plugin system is something that is done post-compilation, what's generated by build.rs from dojo-test-utils is enough to run tests. But this is done inside the dojo-bindgen crate.
Inside sozo, it's currently not possible to decouple the build from the plugin system being invoked after compilation. For this reason, if the time to execute the test suite must remain reasonable, we can remove this test for now.
However, when the dry-run option will be available, we could imagine sozo outputting the migration strategy + the plugins that are expected to run. Like this, sozo tests cover the plugin integration, but not the actual bindings generation (which is expected to be tested in dojo-bindgen).
Sounds reasonable to you guys?
|
(ci failure looks unrelated) |
AccountOptionsto be consistent with other placesAccountOptionsBuildArgsMigrateArgs::run