Foundry test suite clean up

[maurelian]

Description

Furthers the work of reorganizing the forge tests, which was begun in #3611.

Test organizing principles

1. Uses contracts to organize the test suite similar to how mocha uses describe.
2. Every non-trivial state changing function should have a contract for happy and sad path tests. This helps to make it very obvious where there are not sad tests (I haven't added any new tests, so there are quite a few empty placeholder contracts).
3. Simpler functions like getters and setters are grouped together into test contracts.
4. All contracts and functions should be consistently named following a convention.

Test naming convention

Test function names are split by underscores, into 3 or 4 parts. An example function name is test_onlyOwner_callerIsNotOwner_reverts().

The parts are: [method]_[FunctionName]_[reason]_[success], where:

1. [method] is either test or testFuzz
2. [FunctionName] is the name of the function or higher level behavior being tested.
3. [reason] is an optional description for the behavior being tested.
4. [success] must be one of success or reverts or _fails.

Name validation

I also wrote a quick and dirty script which can be run to validate function names and identify ones which are not yet compliant. From within the bedrock package:
ts-node scripts/forges-test-names.ts.

It will error, because there are still test files which haven't yet been cleaned up.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 55306da

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[mergify]

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

[mergify]

Hey @maurelian! This PR has merge conflicts. Please fix them before continuing review.

[smartcontracts]

I really like this refactor. It makes holes in test coverage much more obvious. Thanks!

[tynes]

Why is this included if there are none?

[maurelian]

The intention is to express that I've actually thought about whether or not there are any sad path tests that could be written here, and determined that there are none.
I like having it, but I'm not attached to it.

[tynes]

Should this script run as part of CI?

[maurelian]

eventually yes, but I haven't yet gone through all the test files.

[maurelian]

It would fail if run in CI now.

[tynes]

Generally makes sense, I think we need to document the guidelines in the readme that you posted here

Also this PR needs a rebase to fix the conflicts with the more files

[maurelian]

@tynes
I documented the testing conventions here: a61feaa (#3830)

[mergify]

Hey @maurelian! This PR has merge conflicts. Please fix them before continuing review.

[mergify]

Hey @maurelian! This PR has merge conflicts. Please fix them before continuing review.

[maurelian]

Not a priority right now. Setting to draft.

[github-actions]

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.