Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Improve error message in tests #3775

Merged
merged 2 commits into from
Dec 11, 2023
Merged

Conversation

kevaundray
Copy link
Contributor

Description

@jfecher noted that occassionally the JSON conversion method would fail. This changes expect to unwrap_or_else so that we know exactly what stdout input the error is failing on.

Problem*

Resolves

Summary*

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [Exceptional Case] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@jfecher
Copy link
Contributor

jfecher commented Dec 11, 2023

This happened because of the println that was still in that PR causing the JSON to no longer parse

@kevaundray kevaundray force-pushed the kw/add-better-error-message branch from 6390a7e to 4f4e61e Compare December 11, 2023 19:44
@TomAFrench TomAFrench added this pull request to the merge queue Dec 11, 2023
Merged via the queue into master with commit fa93aa7 Dec 11, 2023
33 checks passed
@TomAFrench TomAFrench deleted the kw/add-better-error-message branch December 11, 2023 20:20
AztecBot pushed a commit that referenced this pull request Jan 3, 2024
Ok. Don't be scared. 262 files sounds horrific but it's mostly path
updates and that kind of nonsense. The `noir-contracts` and
`noir-compiler` changes are probably what to focus on.

* We update boxes to use our build of nargo, and the modified
code-generator.
* We update paths in docs, as noir-contracts/src/contracts moved to
noir-contracts/contracts, as src is now pure codegen output.
* Contracts are now imported e.g. `import { ChildContractArtifact } from
'@aztec/noir-contracts/Child';`. You can still just import from top
level index, but it's pretty cruel to ask the runtime to parse all the
artifacts just to get one, they are huge.
* Contract files are now just named as per the name of the contract
(i.e. not snake case). Less moving parts is better here. Given it's
codegen output it's acceptable to allow the output names to be
inconsistent.
* aztec.js is now responsible for copying the account contracts into
itself, as opposed to have some other random module push code into it.
But we just need to get rid of this baked account stuff at some point
anyway.
* Got rid of lodash.zip in one place, and then restrained myself to not
go further. But think we should remove the "trivial" lodash cases at
some point.
* Tidied up yp/bootstrap a bit, it's basically in line with the
dockerfile at this point. Will prob make dockerfile just call bootstrap
as part of some other docker cleanup I'll do later.
* `source-map-support` in cli.
* Remove compile command from cli. We are just going to promote use of
aztec-nargo.
* The ts and noir generators now expect nargo output as input, rather
than our transformed abi. The ts generator outputs the transformed abi
as part of it's generation.
* Delete all the script stuff from `noir-contracts`. src folder is now
just the codegen output, and the codegen is done with a trivial script
to call compile and the ts generator in noir-compiler.
* Added an unused script called `transform_json_abi.sh` that uses a tiny
bit of jq to perform the transform. Probably to be deleted, especially
if we just stop transforming the noir output and use it directly, but it
served me as a useful tool at one point.
sirasistant pushed a commit that referenced this pull request Jan 4, 2024
Ok. Don't be scared. 262 files sounds horrific but it's mostly path
updates and that kind of nonsense. The `noir-contracts` and
`noir-compiler` changes are probably what to focus on.

* We update boxes to use our build of nargo, and the modified
code-generator.
* We update paths in docs, as noir-contracts/src/contracts moved to
noir-contracts/contracts, as src is now pure codegen output.
* Contracts are now imported e.g. `import { ChildContractArtifact } from
'@aztec/noir-contracts/Child';`. You can still just import from top
level index, but it's pretty cruel to ask the runtime to parse all the
artifacts just to get one, they are huge.
* Contract files are now just named as per the name of the contract
(i.e. not snake case). Less moving parts is better here. Given it's
codegen output it's acceptable to allow the output names to be
inconsistent.
* aztec.js is now responsible for copying the account contracts into
itself, as opposed to have some other random module push code into it.
But we just need to get rid of this baked account stuff at some point
anyway.
* Got rid of lodash.zip in one place, and then restrained myself to not
go further. But think we should remove the "trivial" lodash cases at
some point.
* Tidied up yp/bootstrap a bit, it's basically in line with the
dockerfile at this point. Will prob make dockerfile just call bootstrap
as part of some other docker cleanup I'll do later.
* `source-map-support` in cli.
* Remove compile command from cli. We are just going to promote use of
aztec-nargo.
* The ts and noir generators now expect nargo output as input, rather
than our transformed abi. The ts generator outputs the transformed abi
as part of it's generation.
* Delete all the script stuff from `noir-contracts`. src folder is now
just the codegen output, and the codegen is done with a trivial script
to call compile and the ts generator in noir-compiler.
* Added an unused script called `transform_json_abi.sh` that uses a tiny
bit of jq to perform the transform. Probably to be deleted, especially
if we just stop transforming the noir output and use it directly, but it
served me as a useful tool at one point.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants