-
Notifications
You must be signed in to change notification settings - Fork 45
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.
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
adds test for token list fetching #361
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Love tests. One suggestion to remove lint ignores.
src/__tests__/tokenList.test.ts
Outdated
const resultingTokens = await fetchTokenList(1); | ||
// eslint-disable-next-line @typescript-eslint/no-unused-expressions | ||
expect(resultingTokens).to.be.empty; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you avoid the lint ignore by putting the await statement directly into the expect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem here is that the to.be.empty is not a function so it looks like a normal expression which is not used.
But in fact this actually does something (checking if the object ist empty). There are other tests with the same issue.
I found a more elegant solution which disabled this rule for all test files by including this in the eslint config:
{
"overrides": [
{
"files": ["*.test.ts", "*.test.tsx"],
"rules": {
"no-unused-expressions": "off"
}
}
]
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems legit... go for it!
We use chai for testing and the empty check is always a false positive finding.
Looks ready! |
Just a small test case which would have found the error while fetching / parsing the token list.