Skip to content

Commit

Permalink
foundry test function fix
Browse files Browse the repository at this point in the history
  • Loading branch information
dbale-altoros committed Aug 16, 2023
1 parent 48258ca commit 0b396de
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 8 deletions.
2 changes: 1 addition & 1 deletion docs/rules/naming/foundry-test-functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ This rule accepts an array of options:
- Supported Regex: ```test(Fork)?(Fuzz)?(Fail)?_(Revert(If_|When_){1})?\w{1,}```
- This rule should be executed in a separate folder with a separate .solhint.json => ```solhint --config .solhint.json testFolder/**/*.sol```
- This rule applies only to `external` and `public` functions
- This rule skips the `setUp()` function
- This rule skips the `setUp()` function by default

## Examples
### 👍 Examples of **correct** code for this rule
Expand Down
16 changes: 13 additions & 3 deletions lib/common/identifier-naming.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,18 @@ module.exports = {
return text && text[0] === '_'
},

isNotFoundryTestCase(text) {
const regex = /^test(Fork)?(Fuzz)?(Fail)?_(Revert(If_|When_){1})?\w{1,}$/
return !match(text, regex)
isFoundryTestCase(text) {
// this one checks CamelCase after test keyword
// const regexTest = /^test(Fork)?(Fuzz)?(Fail)?(_)?[A-Z](Revert(If_|When_){1})?\w{1,}$/

const regexTest = /^test(Fork)?(Fuzz)?(Fail)?(_)?(Revert(If_|When_){1})?\w{1,}$/
const matchRegexTest = match(text, regexTest)

// this one checks CamelCase after test keyword
// const regexInvariant = /^(invariant|statefulFuzz)(_)?[A-Z]\w{1,}$/
const regexInvariant = /^(invariant|statefulFuzz)(_)?\w{1,}$/
const matchRegexInvariant = match(text, regexInvariant)

return matchRegexTest || matchRegexInvariant
},
}
4 changes: 2 additions & 2 deletions lib/rules/naming/foundry-test-functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const meta = {
note: 'This rule applies only to `external` and `public` functions',
},
{
note: 'This rule skips the `setUp()` function',
note: 'This rule skips the `setUp()` function by default',
},
],
},
Expand All @@ -79,7 +79,7 @@ class FoundryTestFunctionsChecker extends BaseChecker {
!this.searchInArray(this.skippedFunctions, node.name) &&
(node.visibility === 'public' || node.visibility === 'external')
) {
if (naming.isNotFoundryTestCase(node.name)) {
if (!naming.isFoundryTestCase(node.name)) {
this.error(node, `Function ${node.name}() must match Foundry test naming convention`)
}
}
Expand Down
11 changes: 9 additions & 2 deletions test/rules/naming/foundry-test-functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const contractWith = require('../../common/contract-builder').contractWith
const { assertErrorCount, assertNoErrors, assertErrorMessage } = require('../../common/asserts')

const ALLOWED_FUNCTION_NAMES = [
'test',
'test_',
'testFork_',
'testFuzz_',
Expand All @@ -20,9 +21,14 @@ const ALLOWED_FUNCTION_NAMES = [
'testFuzz_Revert_',
'testFuzz_If_',
'testFuzz_When_',
'invariant',
'invariant_',
'invariantA',
'statefulFuzz',
'statefulFuzz_',
]

const DISALLOWED_FUNCTION_NAMES = ['Test_', 'Test', 'test', 'testo_', '', 'any', 'setUp', 'other']
const DISALLOWED_FUNCTION_NAMES = ['Test_', 'Test', '', 'any', 'setUp', 'other', '_']

const composeFunctionName = (prefix, name, visibility) =>
'function ' + prefix + name + ' ' + visibility + ' { testNumber = 42; }'
Expand All @@ -33,6 +39,7 @@ describe('Linter - foundry-test-functions', () => {
const functionDefinition = composeFunctionName(prefix, 'FunctionName()', 'public')
const code = contractWith(functionDefinition)

console.log('`code` :>> ', code)
const report = linter.processStr(code, {
rules: { 'foundry-test-functions': ['error', ['setUp', 'finish']] },
})
Expand Down Expand Up @@ -171,7 +178,7 @@ describe('Linter - foundry-test-functions', () => {
)
})

it(`should NOT raise error only for setUp when rule is just on 'error'`, () => {
it(`should NOT raise error only for setUp when rule is just on 'error' (setUp is default)`, () => {
const code = contractWith(`
function setUp() public { testNumber = 42; }
function finish() public { testNumber = 43; }
Expand Down

0 comments on commit 0b396de

Please sign in to comment.