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.

Sign up for GitHub

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

deps(testing) - consolidate testing frameworks across the project #819

Merged
merged 8 commits into from
Nov 11, 2023
Merged

deps(testing) - consolidate testing frameworks across the project #819

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
399ac31
deps(testing) - consolidate testing frameworks across the project
ctrlc03 Nov 7, 2023
259c14b
deps(testing) - allow more timeout during MaciState tests to ensure t…
ctrlc03 Nov 8, 2023
1329572
deps(testing) - allow more timeout across all tests to ensure they ha…
ctrlc03 Nov 8, 2023
e14bebe
deps(mocha) - fix integration tests by changing filename and ensuring…
ctrlc03 Nov 9, 2023
8a14264
deps(mocha) - fix integration tests errors due to chai eq vs deep.eq …
ctrlc03 Nov 10, 2023
f2a8abf
Remove mention of Jest from README
samajammin Nov 10, 2023
e576c84
Add info on Mocha and Chai
samajammin Nov 10, 2023
f44ff8e
Update test instructions
samajammin Nov 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/reusable-e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
id: check_changes
run: |
CHANGED_FILES=${{ steps.get-changed-files.outputs.all }}
if echo "$CHANGED_FILES" | grep -q "circuit" && ! echo "$CHANGED_FILES" | grep -q ".github/workflows/circuit-build.yml"; then
if echo "$CHANGED_FILES" | grep -q "\.circom"; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this change about?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yea sorry for not mentioning in the description! Basically this change is so that the e2e test only run when the actual circom files change, not any files in the circuits folder as it was until this commit. I was planning to introduce this with another PR but got carried away by not wanting to wait on circuits compilation for this PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Great catch.

echo "CHANGED=true" >> $GITHUB_ENV
echo "Circuits have changes."
else
Expand Down
16 changes: 12 additions & 4 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,26 @@ own unit tests.

### Unit tests

Unit tests within the project are built using [Mocha](https://mochajs.org/) and [Chai](https://www.chaijs.com/). Mocha is a test framework that provides the environment to write and run JavaScript tests, while Chai is an assertion library that allows us to write assertions in a more expressive and readable way.


The following submodules contain unit tests: `core`, `crypto`, `circuits`,
`contracts`, and `domainobjs`.

Except for the `contracts` submodule, run unit tests as such (the following
samajammin marked this conversation as resolved.
Show resolved Hide resolved
example is for `crypto`):
You can run all unit tests from the root directory of the repo by running:

```bash
npm run test
```

Our you can run unit tests within each submodule. for example to run the `crypto` tests:

```bash
cd crypto
npm run test
```

<!-- TODO: confirm if true -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ctrlc03 is this necessary? Seems I'm able to run the full suite e.g. of contracts tests without nonce errors...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, start a Hardhat instance in a separate terminal:

cd contracts
npm run hardhat

Is this needed? I'm able to just run npm run test within contracts & it seems to automatically start up Hardhat

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea that's how it's supposed to work (hardhat does start itself) so yea feel free to remove. In general most of these README are outdated (wondering if it might better to address all in one PR?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general most of these README are outdated (wondering if it might better to address all in one PR?)

I'm alright with that, though generally I think best to update documentation at the same time as the code is updated (ideally in the same PR), because:

  • that tends to make it easier for PR creator, given the specific info & context is fresh in their mind
  • it also makes it easier for the reviewer, so they can easily understand what's changing
  • it avoids the issue of having this big chore for someone to pick up at the end of this refactoring effort. Not to mention if the chore drops as a priority, then everything is a mess 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted! I'll make sure from now on all doc updates are included in the PR 🙂

For `contracts` and `integrationTests`, run the tests one by one. This prevents
incorrect nonce errors.

Expand All @@ -136,12 +145,11 @@ cd contracts
npm run test
```

Or run tests individual as such:
Or run individual tests:

```bash
cd contracts
npm run test-accQueue
npx jest AccQueue
```

Where both test commands run `AccQueue.test.ts`
Expand Down
29 changes: 0 additions & 29 deletions circuits/jest.config.js
View file
Open in desktop

This file was deleted.

Loading
Loading