-
Notifications
You must be signed in to change notification settings - Fork 2
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
sonarcloud bug complain fix #8
Conversation
tests/04_00_02-fedchange.js
Outdated
@@ -281,14 +281,14 @@ describe('RSK Federation change', function() { | |||
await checkFederationAddedNewKeys(); | |||
var addResult = await rskClientOldFed.rsk.bridge.methods.addFederatorPublicKeyMultikey('0xaabb', '0xccdd', '0xeeff').call({ from: getRandomFedChangeAddress() }); |
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.
var addResult = await rskClientOldFed.rsk.bridge.methods.addFederatorPublicKeyMultikey('0xaabb', '0xccdd', '0xeeff').call({ from: getRandomFedChangeAddress() }); | |
const addResultFirstCall = await rskClientOldFed.rsk.bridge.methods.addFederatorPublicKeyMultikey('0xaabb', '0xccdd', '0xeeff').call({ from: getRandomFedChangeAddress() }); | |
expect(Number(addResult)).to.equal(1); // First call successful |
Sonar is complaining because you declare this variable here but never use it, then declare it again as const.
I think the point of the test is to make the call twice, and check that the second time it fails. So we could also check the result of the first call
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.
ohh i fixed it before i read this comment, hope my naming is ok for you :)
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.
Naming is fine, but I would change it to const and also assert the result the first time
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.
hmm i'm not sure i understand, the assert was always there...
expect(Number(addKeysResult)).to.equal(-10); // not a public key
tests/04_00_02-fedchange.js
Outdated
await rskClientOldFed.rsk.bridge.methods.addFederatorPublicKeyMultikey( | ||
|
||
const addResult = await rskClientOldFed.rsk.bridge.methods.addFederatorPublicKeyMultikey( |
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.
const addResult = await rskClientOldFed.rsk.bridge.methods.addFederatorPublicKeyMultikey( | |
const addResultSecondCall = await rskClientOldFed.rsk.bridge.methods.addFederatorPublicKeyMultikey( |
pipeline: run |
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.
LGTM.
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
No description provided.