feat(cli): auto-recompiling when aztec test is run#20729
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
4fe024b to
0ce0067
Compare
c23a37d to
05bbbf0
Compare
7a2fec1 to
96b2f70
Compare
f9b122d to
fc7e946
Compare
277bf39 to
54a6fc1
Compare
d3a3170 to
11d8d00
Compare
54a6fc1 to
b16a65d
Compare
59a1df5 to
a12d173
Compare
ee08542 to
c70a2ba
Compare
I agree with your reasoning 100%. The
I believe part of the reason to implement it in bash was to skip loading all of the TS. Did you not notice a huge slowdown due to the importing of ts modules etc? I do agree with bash being a terrible way to do things, but because the CLI is currently huge this is a problem today. The real solution would be to split the CLI (or to split commands) so that compile test etc are all lightweight. |
@nventuro I just did measurements on the default workspace tests and the version with the check is like 0.3s slower than version without it. Given that to run it all takes like 5s this doesn't seem to be a problem. |
Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
nventuro
left a comment
There was a problem hiding this comment.
Lovely.
WE HAVE AUTORECOMPILE ON TEST YAY
In this PR I ensure that contracts get automatically recompiled when `aztec test` is run. To avoid unnecessary compilations I use `make` command inspired approach of checking timestamp of source files (*.toml and *.nr files) against the timestamp of the oldest artifact in the `/target` dir (as artifact I consider any `*.json` file). **Note:** By using the oldest timestamp we would get constant recompilation on each run in case the dev placed there some random `*.json` file. **Question**: Does the reviewer think this ^ is fine? Alternatively I could nuke the `target` dir whenever a recompilation is being run. This feels a bit dangerous though. Another approach would be to predict the artifact name of individual crates and just check against those. This would be the most efficient but it introduces complexity and brittleness in case `nargo` artifact names change. I think I would keep it as it is for now and come up with solution if someone complains about constant recompilations. I initially thought I will implement this in the `aztec test` command but that command is fully implemented in bash. When I had AI implement this the resulting bash was impossible to comprehend so I told it to rewrite in typescript. Given that the `aztec test` command is calling `aztec compile` that is implemented in typescript it made sense for this PR to modify only that command. This has the added advantage that it will make bugs in this code way more visible because when a dev runs `aztec compile` they will either see that the code is getting re-compiled or they will get "No source changes detected, skipping compilation." log. If they modify a contract, run `aztec compile` and see that nothing got compiled it will be obvious that the thing is broken. Closes https://linear.app/aztec-labs/issue/F-197/auto-recompile-contracts-on-tests --------- Co-authored-by: benesjan <benesjan@users.noreply.github.com> Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>
## Summary Backports 6 PRs with aztec CLI improvements from `next` to `v4-next`: - #20681 - refactor: aztec new and init creating 2 crates - #20711 - test: aztec new scaffold works - #20723 - feat(cli): warning if contract crate has tests - #20729 - feat(cli): auto-recompiling when aztec test is run - #21007 - feat: aztec new supporting multiple contract crates - #21245 - feat: asserts that aztec dep version matches cli
|
Backported to v4-next in #22587 |
BEGIN_COMMIT_OVERRIDE fix(pxe): cap event filter toBlock to last synced block (#22573) fix(pxe): round tx expiration timestamp to reduce precision (#22577) fix: eliminate anvil watcher warp race and false success logs (#22584) refactor: aztec new and init creating 2 crates (#20681) test: aztec new scaffold works (#20711) feat(cli): warning if contract crate has tests (#20723) feat(cli): auto-recompiling when aztec test is run (#20729) feat: aztec new supporting multiple contract crates (#21007) feat: asserts that aztec dep version matches cli (#21245) chore: backport aztec CLI improvements to v4-next (#22587) feat: check noir release has nargo binaries before releasing (#22551) chore: cache chainInfo in embeddedwallet (#22592) fix: wrap external getCapsule in transactionAsync (#22595) fix(pxe): throw clear error for invalid comparator in pick_notes (#22585) refactor(aztec-nr): rename conversion fns to encode_/decode_ naming (#22576) feat: infrastructure for testing `[new_contract_artfiacts, old_aztec_stack]` (#22593) chore: fix unnecessary and inconsistent side-effect counter increments (#22245) fix: update FaceID wallet redirects and strip anchors in redirect validation (#22505) docs: add getting started on testnet guide (#22366) docs: add getting started on testnet guide (backport #22366) (#22619) feat(aztec-nr): new BoundedVec emit private log APIs (#22064) END_COMMIT_OVERRIDE

In this PR I ensure that contracts get automatically recompiled when
aztec testis run. To avoid unnecessary compilations I usemakecommand inspired approach of checking timestamp of source files (*.toml and *.nr files) against the timestamp of the oldest artifact in the/targetdir (as artifact I consider any*.jsonfile).Note: By using the oldest timestamp we would get constant recompilation on each run in case the dev placed there some random
*.jsonfile.Question: Does the reviewer think this ^ is fine? Alternatively I could nuke the
targetdir whenever a recompilation is being run. This feels a bit dangerous though. Another approach would be to predict the artifact name of individual crates and just check against those. This would be the most efficient but it introduces complexity and brittleness in casenargoartifact names change. I think I would keep it as it is for now and come up with solution if someone complains about constant recompilations.I initially thought I will implement this in the
aztec testcommand but that command is fully implemented in bash. When I had AI implement this the resulting bash was impossible to comprehend so I told it to rewrite in typescript. Given that theaztec testcommand is callingaztec compilethat is implemented in typescript it made sense for this PR to modify only that command. This has the added advantage that it will make bugs in this code way more visible because when a dev runsaztec compilethey will either see that the code is getting re-compiled or they will get "No source changes detected, skipping compilation." log. If they modify a contract, runaztec compileand see that nothing got compiled it will be obvious that the thing is broken.Closes https://linear.app/aztec-labs/issue/F-197/auto-recompile-contracts-on-tests