From a3b6d930ecee77c4f4c6416439b007750f3880c7 Mon Sep 17 00:00:00 2001 From: SidestreamColdMelon Date: Mon, 18 Nov 2024 14:57:03 +0100 Subject: [PATCH 1/4] add section with test coverage rules for the new modules --- spell/spell-crafter-mainnet-workflow.md | 6 +++++- spell/spell-reviewer-mainnet-checklist.md | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/spell/spell-crafter-mainnet-workflow.md b/spell/spell-crafter-mainnet-workflow.md index c507146..48eff29 100644 --- a/spell/spell-crafter-mainnet-workflow.md +++ b/spell/spell-crafter-mainnet-workflow.md @@ -126,9 +126,13 @@ Repo: https://github.com/makerdao/spells-mainnet * [ ] Test DAI/MKR streams and payments, lerps * [ ] Test the sum of all DAI/MKR payments matches the Exec Sheet * Run tests via `make test` (or `make test match=` to inspect debug traces) - * [ ] Ensure good coverage (every spell action is tested) + - [ ] Ensure good coverage (every spell action is tested) * [ ] Ensure every test function is declared as `public` * [ ] IF the test needs to run, it MUST NOT have the `skipped` modifier; OTHERWISE, it MUST have the `skipped` modifier + - Testing of modules initialised via the spell + - [ ] Sanity checks of the constructor arguments + - [ ] Sanity checks of all values added/updated by the spell/init function + - [ ] End-to-end "happy path" interaction with the module * [ ] Tests PASS via `make test` * [ ] Ensure `DssExecLib` address used in current spell (`DssExecLib.address`) matches `dss-exec-lib` [Latest Release Tag](https://github.com/makerdao/dss-exec-lib/releases/latest) * [ ] Push committed content to already opened PR diff --git a/spell/spell-reviewer-mainnet-checklist.md b/spell/spell-reviewer-mainnet-checklist.md index 3b2a52b..e585b24 100644 --- a/spell/spell-reviewer-mainnet-checklist.md +++ b/spell/spell-reviewer-mainnet-checklist.md @@ -314,6 +314,10 @@ * [ ] Ensure each spell action has sufficient test coverage _List actions for which coverage was checked here_ * [ ] Ensure that any other env variable does not affect execution of the tests (for example, by inspecting the output of `printenv | grep "FOUNDRY_\|DAPP_"`) + - Testing of modules initialised via the spell + - [ ] Sanity checks of the constructor arguments + - [ ] Sanity checks of all values added/updated by the spell/init function + - [ ] End-to-end "happy path" interaction with the module * [ ] Check all tests are passing locally using `make test` * [ ] Ensure every test listed in the _coverage_ item above is present in the logs and with the `[PASS]` prefix. From b5039d339d59c871d461f771ac2379dfa58365ed Mon Sep 17 00:00:00 2001 From: SidestreamColdMelon Date: Mon, 18 Nov 2024 15:15:15 +0100 Subject: [PATCH 2/4] use * instead of dash --- spell/spell-crafter-mainnet-workflow.md | 10 +++++----- spell/spell-reviewer-mainnet-checklist.md | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/spell/spell-crafter-mainnet-workflow.md b/spell/spell-crafter-mainnet-workflow.md index 48eff29..64d82f4 100644 --- a/spell/spell-crafter-mainnet-workflow.md +++ b/spell/spell-crafter-mainnet-workflow.md @@ -126,13 +126,13 @@ Repo: https://github.com/makerdao/spells-mainnet * [ ] Test DAI/MKR streams and payments, lerps * [ ] Test the sum of all DAI/MKR payments matches the Exec Sheet * Run tests via `make test` (or `make test match=` to inspect debug traces) - - [ ] Ensure good coverage (every spell action is tested) + * [ ] Ensure good coverage (every spell action is tested) * [ ] Ensure every test function is declared as `public` * [ ] IF the test needs to run, it MUST NOT have the `skipped` modifier; OTHERWISE, it MUST have the `skipped` modifier - - Testing of modules initialised via the spell - - [ ] Sanity checks of the constructor arguments - - [ ] Sanity checks of all values added/updated by the spell/init function - - [ ] End-to-end "happy path" interaction with the module + * Testing of modules initialised via the spell + * [ ] Sanity checks of the constructor arguments + * [ ] Sanity checks of all values added/updated by the spell function + * [ ] End-to-end "happy path" interaction with the module * [ ] Tests PASS via `make test` * [ ] Ensure `DssExecLib` address used in current spell (`DssExecLib.address`) matches `dss-exec-lib` [Latest Release Tag](https://github.com/makerdao/dss-exec-lib/releases/latest) * [ ] Push committed content to already opened PR diff --git a/spell/spell-reviewer-mainnet-checklist.md b/spell/spell-reviewer-mainnet-checklist.md index e585b24..e91e5b6 100644 --- a/spell/spell-reviewer-mainnet-checklist.md +++ b/spell/spell-reviewer-mainnet-checklist.md @@ -314,10 +314,10 @@ * [ ] Ensure each spell action has sufficient test coverage _List actions for which coverage was checked here_ * [ ] Ensure that any other env variable does not affect execution of the tests (for example, by inspecting the output of `printenv | grep "FOUNDRY_\|DAPP_"`) - - Testing of modules initialised via the spell - - [ ] Sanity checks of the constructor arguments - - [ ] Sanity checks of all values added/updated by the spell/init function - - [ ] End-to-end "happy path" interaction with the module + * Testing of modules initialised via the spell + * [ ] Sanity checks of the constructor arguments + * [ ] Sanity checks of all values added/updated by the spell/init function + * [ ] End-to-end "happy path" interaction with the module * [ ] Check all tests are passing locally using `make test` * [ ] Ensure every test listed in the _coverage_ item above is present in the logs and with the `[PASS]` prefix. From 3df27ecb3936ba0f4b87e823bc93c8591b3e073e Mon Sep 17 00:00:00 2001 From: SidestreamColdMelon Date: Mon, 18 Nov 2024 15:15:45 +0100 Subject: [PATCH 3/4] fix CI --- spell/spell-reviewer-mainnet-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spell/spell-reviewer-mainnet-checklist.md b/spell/spell-reviewer-mainnet-checklist.md index e91e5b6..63d8f65 100644 --- a/spell/spell-reviewer-mainnet-checklist.md +++ b/spell/spell-reviewer-mainnet-checklist.md @@ -316,7 +316,7 @@ * [ ] Ensure that any other env variable does not affect execution of the tests (for example, by inspecting the output of `printenv | grep "FOUNDRY_\|DAPP_"`) * Testing of modules initialised via the spell * [ ] Sanity checks of the constructor arguments - * [ ] Sanity checks of all values added/updated by the spell/init function + * [ ] Sanity checks of all values added/updated by the spell function * [ ] End-to-end "happy path" interaction with the module * [ ] Check all tests are passing locally using `make test` * [ ] Ensure every test listed in the _coverage_ item above is present in the logs and with the `[PASS]` prefix. From 5a5ca3429b17d89af9e1bd643bb0fb4c2bd69cb4 Mon Sep 17 00:00:00 2001 From: SidestreamColdMelon Date: Wed, 27 Nov 2024 11:09:57 +0100 Subject: [PATCH 4/4] use IF statement --- spell/spell-crafter-mainnet-workflow.md | 2 +- spell/spell-reviewer-mainnet-checklist.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spell/spell-crafter-mainnet-workflow.md b/spell/spell-crafter-mainnet-workflow.md index 64d82f4..bd184bc 100644 --- a/spell/spell-crafter-mainnet-workflow.md +++ b/spell/spell-crafter-mainnet-workflow.md @@ -129,7 +129,7 @@ Repo: https://github.com/makerdao/spells-mainnet * [ ] Ensure good coverage (every spell action is tested) * [ ] Ensure every test function is declared as `public` * [ ] IF the test needs to run, it MUST NOT have the `skipped` modifier; OTHERWISE, it MUST have the `skipped` modifier - * Testing of modules initialised via the spell + * IF a new module is initialized via the spell, the tests must include * [ ] Sanity checks of the constructor arguments * [ ] Sanity checks of all values added/updated by the spell function * [ ] End-to-end "happy path" interaction with the module diff --git a/spell/spell-reviewer-mainnet-checklist.md b/spell/spell-reviewer-mainnet-checklist.md index 63d8f65..2b9f472 100644 --- a/spell/spell-reviewer-mainnet-checklist.md +++ b/spell/spell-reviewer-mainnet-checklist.md @@ -314,7 +314,7 @@ * [ ] Ensure each spell action has sufficient test coverage _List actions for which coverage was checked here_ * [ ] Ensure that any other env variable does not affect execution of the tests (for example, by inspecting the output of `printenv | grep "FOUNDRY_\|DAPP_"`) - * Testing of modules initialised via the spell + * IF a new module is initialized via the spell, the tests must include * [ ] Sanity checks of the constructor arguments * [ ] Sanity checks of all values added/updated by the spell function * [ ] End-to-end "happy path" interaction with the module