Skip to content

Commit c0ffee7

Browse files
chore: update process
1 parent c0ffee6 commit c0ffee7

1 file changed

Lines changed: 51 additions & 53 deletions

File tree

sites/wonderland/docs/testing/campaign-processes.md

Lines changed: 51 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ We have built these guidelines for every Wonderland member to follow. As we are
88

99
## Responsibilities: who does it?
1010

11-
### Continuous Quality Assurance (QA), which includes:
11+
### Continuous Quality Assurance (QA), which includes
1212

1313
- **Unit Tests**
1414
- **Integration Tests**
@@ -30,38 +30,38 @@ Sliding responsibility, based on the timeline (research/design team, then team l
3030
## Timeline: when do we do it?
3131

3232
- Before development:
33-
- Collect our first protocol properties
33+
- Collect our first protocol properties
3434
- During development:
35-
- Write unit tests
36-
- Write integration tests
37-
- Update protocol properties
35+
- Write unit tests
36+
- Write integration tests
37+
- Update protocol properties
3838
- At the end of development:
39-
- Internal review
40-
- Advanced testing:
41-
- Finish collecting properties
42-
- Fuzzing campaigns
43-
- (Symbolic execution)
39+
- Internal review
40+
- Advanced testing:
41+
- Finish collecting properties
42+
- Fuzzing campaigns
43+
- (Symbolic execution)
4444

4545
## Guidelines: how do we do it?
4646

47-
### Design Phase:
47+
### Design Phase
4848

4949
While the project is still being defined and refined, we start preparing the advanced tests by collecting some invariants. This initial collection is often a first draft, as many of these invariants might still change during the development.
5050

5151
### Implementation Phase
5252

5353
- **Unit Tests (QA)**
54-
- Unit tests *must* be implemented continuously, cross-testing is recommended, with developer A implementing the logic and developer B writing the unit tests.
55-
- Aim for path coverage if the logic has few independent conditions; otherwise, branch coverage is preferred.
56-
- Tools like Bulloak *might* be used for listing branches/conditions to test.
57-
- Good unit tests *must* have a comprehensive branch test in addition to a high coverage, tending toward 100% by the end of the implementation phase.
58-
- CI *might* have an action to enforce a non-inferior branch coverage on new PRs.
59-
- Tests which are not implemented *must* use `vm.skip(true)`, and must NOT be commented out or left as an empty body. The linter should prohibit empty blocks in tests.
60-
- Unit tests *must* be solitary tests, testing only one part of the logic and mocking the rest.
61-
- The whole range of values leading to the same behaviour should be used in fuzzed parameters.
62-
- Only one path should be tested at a time, especially when using fuzzed inputs.
63-
- Forge being efficient, the number of runs made by the fuzzer shouldn't be reduced below the default 1000 (if tests are taking too long to run with this value, then a design fault should be suspected)
64-
- Use forge-std's `bound` when there is more than a single value to exclude from the fuzzed value. `vm.assume` should be avoided as much as possible, or only to exclude a single value, like `address(0)` for instance (attention should be made to multiple "hidden" assumes)
54+
- Unit tests *must* be implemented continuously, cross-testing is recommended, with developer A implementing the logic and developer B writing the unit tests.
55+
- Aim for path coverage if the logic has few independent conditions; otherwise, branch coverage is preferred.
56+
- Tools like Bulloak *might* be used for listing branches/conditions to test.
57+
- Good unit tests *must* have a comprehensive branch test in addition to a high coverage, tending toward 100% by the end of the implementation phase.
58+
- CI *might* have an action to enforce a non-inferior branch coverage on new PRs.
59+
- Tests which are not implemented *must* use `vm.skip(true)`, and must NOT be commented out or left as an empty body. The linter should prohibit empty blocks in tests.
60+
- Unit tests *must* be solitary tests, testing only one part of the logic and mocking the rest.
61+
- The whole range of values leading to the same behaviour should be used in fuzzed parameters.
62+
- Only one path should be tested at a time, especially when using fuzzed inputs.
63+
- Forge being efficient, the number of runs made by the fuzzer shouldn't be reduced below the default 1000 (if tests are taking too long to run with this value, then a design fault should be suspected)
64+
- Use forge-std's `bound` when there is more than a single value to exclude from the fuzzed value. `vm.assume` should be avoided as much as possible, or only to exclude a single value, like `address(0)` for instance (attention should be made to multiple "hidden" assumes)
6565

6666
**Example:**
6767

@@ -93,32 +93,32 @@ function test_foo(uint256 inputToTest) external {
9393
> Note: This example would show a case where using vm. Assuming with low runs can lead to incorrect conclusions about the non-existence of multiples of 10.
9494
9595
- **Integration Tests (QA)**:
96-
- They *should* be conducted during development (top-down with stubs/bottom-up with drivers), this avoids having a test debt at the end of the project and helps find potential high-level bugs along the way.
97-
- If the protocol interacts with other external contracts, they should be run on a fork of the relevant network, at a set block height, using `createSelectFork(string calldata urlOrAlias, uint256 block)` or `createSelectFork(string calldata urlOrAlias, bytes32 transaction)`.
98-
- Integration tests should be conducted following user stories, leading from an entry point A to an end-state B. They are therefore stateful and have multiple transactions.
96+
- They *should* be conducted during development (top-down with stubs/bottom-up with drivers), this avoids having a test debt at the end of the project and helps find potential high-level bugs along the way.
97+
- If the protocol interacts with other external contracts, they should be run on a fork of the relevant network, at a set block height, using `createSelectFork(string calldata urlOrAlias, uint256 block)` or `createSelectFork(string calldata urlOrAlias, bytes32 transaction)`.
98+
- Integration tests should be conducted following user stories, leading from an entry point A to an end-state B. They are therefore stateful and have multiple transactions.
9999
- **E2E Tests (QA)**:
100-
- These are considered *optional*,* as they might be less relevant in some projects (for instance, strictly onchain deliverable versus bridge with heavy onchain dependencies)
101-
- They *might* be horizontal E2E from entry point to end-state, or vertical across the stack if significant logic is performed off-chain.
100+
- These are considered *optional*,* as they might be less relevant in some projects (for instance, strictly onchain deliverable versus bridge with heavy onchain dependencies)
101+
- They *might* be horizontal E2E from entry point to end-state, or vertical across the stack if significant logic is performed off-chain.
102102
- **Differential Testing (QA)**: differential testing *should* be performed during refactor (e.g., using Forge snapshot for gas comparison).
103103
- **Slither in CI**: Consider adding Slither to the CI workflow, but be mindful of potential codebase clutter from skip comments. Care is to be taken not to be over-sensitive; better to have a few real positives than a lot of false negatives. Setting on failing on high or medium severities *might* help in this regard.
104104
- **slither-mutate (coming with medusa/echidna) or [vertigo-rs](https://github.com/RareSkills/vertigo-rs)**: both are still in development, it *might* provide an extra safety net, without creating a huge overhead. It *may* be run occasionally against the whole test suite, uncovering untested logic. For instance, after installing slither, just run `slither-mutate path/to/contract/contract.sol-- test-cmd yarn test:unit` or the python invocation of vertigo (the "rs" means RareSkills, not Rust…)
105105

106106
### Pre and during internal review
107107

108108
- **Properties & Invariants Documentation**:
109-
- A (short) brainstorm session *might* be scheduled, to finalize the list of invariants which will be studied
110-
- An actor-based approach *might* be used, to keep the focus on higher level invariants
111-
- The testing team lead *must* compile the properties in a table in `src/test/invariant/PROPERTIES.md`. This file must include at least three columns:
112-
- **Id:** A sequential number.
113-
- **Properties**: Clear, concise English descriptions.
114-
- Covered: An empty check box
115-
- A fourth column *might* be used, for classification purposes:
116-
- **Type**: Categorise using [Certora's categories](https://github.com/Certora/Tutorials/blob/master/06.Lesson_ThinkingProperties/Categorizing_Properties.pdf):
117-
- Valid state: The protocol can only be in valid states based on the spec.
118-
- State transition: Protocol state changes follow specific orders or conditions.
119-
- Variable transition: Protocol variables change in defined ways.
120-
- High-level property: Properties of the system as a whole.
121-
- Unit test: Target specific functions for detailed examination.
109+
- A (short) brainstorm session *might* be scheduled, to finalize the list of invariants which will be studied
110+
- An actor-based approach *might* be used, to keep the focus on higher level invariants
111+
- The testing team lead *must* compile the properties in a table in `src/test/invariant/PROPERTIES.md`. This file must include at least three columns:
112+
- **Id:** A sequential number.
113+
- **Properties**: Clear, concise English descriptions.
114+
- Covered: An empty check box
115+
- A fourth column *might* be used, for classification purposes:
116+
- **Type**: Categorise using [Certora's categories](https://github.com/Certora/Tutorials/blob/master/06.Lesson_ThinkingProperties/Categorizing_Properties.pdf):
117+
- Valid state: The protocol can only be in valid states based on the spec.
118+
- State transition: Protocol state changes follow specific orders or conditions.
119+
- Variable transition: Protocol variables change in defined ways.
120+
- High-level property: Properties of the system as a whole.
121+
- Unit test: Target specific functions for detailed examination.
122122

123123
Example
124124

@@ -129,13 +129,13 @@ Example
129129
| 2 | Transfer between addresses shouldn't modify supply| High level | [ ] |
130130
```
131131

132-
- **Invariant Implementation (QA/AT)**: The lead/advanced tester *must* implement the invariants starting from higher-level to lower-level properties in the `PROPERTIES.md` file. Complex protocols may require deployment tricks for Medusa.
133-
- **Echnidna/Medusa (AT)**: *should* be used for stateful testing, using the coverage (HTML autogenerated) to discover additional properties.
132+
- **Invariant Implementation (QA/AT)**: The lead/advanced tester *must* implement the invariants starting from higher-level to lower-level properties in the `PROPERTIES.md` file.
133+
- **Forge (AT)**: *should* be used for stateful testing, using coverage guidance.
134134
- **Kontrol / Halmos (AT)**: *might* be used for formal verification (symbolic execution or using K formalism)
135135
- Any finished campaign *must* be run for at least a couple of weeks on the dedicated server
136136
- **CI Workflow (AT)**:
137-
- Medusa and Halmos/Kontrol *should* be in the CI workflow, once the advanced tests are finalised.
138-
- Mutation tests *might* be used as testing suite validation tools, granted *the branch coverage is 100%.*
137+
- Forge and Halmos/Kontrol *should* be in the CI workflow, once the advanced tests are finalised.
138+
- Mutation tests *might* be used as testing suite validation tools, granted *the branch coverage is 100%.*
139139

140140
## Repo organisation: How do we keep sanity?
141141

@@ -156,17 +156,15 @@ Example
156156
├── PROPERTIES.md
157157
├── fuzz/
158158
| ├── handlers/
159-
| | ├── HandlerA.sol
160-
| | ├── HandlerAgents.sol
161-
| | └── HandlerParent.sol
162-
| ├── properties/
163-
| | ├── PropertiesA.sol
164-
| | └── PropertiesParent.sol
165-
│ ├── FuzzTest.t.sol
166-
│ ├── Reproducers.t.sol
159+
| | ├── HandlerA.t.sol
160+
| | ├── GhostState.t.sol
161+
| | └── BaseHandler.t.sol
162+
│ ├── Invariant.t.sol
163+
│ ├── HandlersTarget.t.sol
164+
│ ├── ExecutionPaths.t.sol
167165
| └── Setup.sol
168166
└── symbolic/
169167
└── KontrolTest.t.sol
170168
```
171169

172-
Contract name *should* reflect the test technique used, `contract UnitMyContract` for instance.
170+
Contract name *should* reflect the test technique used, `contract UnitMyContract` for instance.

0 commit comments

Comments
 (0)