add more test for fixedFeedbackRate controller implementation #1628
Conversation
There was a problem hiding this comment.
Sorry, this is going to be a hard one to test and make sure that the descriptions of the tests provide detail. Unfortunately the original tests are not ideal either but I don't think we should follow the same pattern as this file if we know they are not ideal. If possible it would be nice to update them but I think that would have to be an exercise for later.
However I saw you correctly used async/await in your tests and the original ones didn't, so a quick update worth doing would be to apply that pattern to the original tests.
packages/caliper-core/test/worker/rate-control/fixedFeedbackRate.js
Outdated
Show resolved
Hide resolved
packages/caliper-core/test/worker/rate-control/fixedFeedbackRate.js
Outdated
Show resolved
Hide resolved
packages/caliper-core/test/worker/rate-control/fixedFeedbackRate.js
Outdated
Show resolved
Hide resolved
packages/caliper-core/test/worker/rate-control/fixedFeedbackRate.js
Outdated
Show resolved
Hide resolved
packages/caliper-core/test/worker/rate-control/fixedFeedbackRate.js
Outdated
Show resolved
Hide resolved
tunedev
force-pushed
the
add-more-test-for-fixedFeedbackRate
branch
from
16508ec
to
2eec183
Compare
packages/caliper-core/test/worker/rate-control/fixedFeedbackRate.js
Outdated
Show resolved
Hide resolved
Signed-off-by: Babatunde Sanusi <swisskid95@gmail.com>
tunedev
force-pushed
the
add-more-test-for-fixedFeedbackRate
branch
from
2eec183
to
7fe92a4
Compare
| txnStats.stats.txCounters.totalSubmitted = 0; | ||
| controller.applyRateControl(); | ||
| it('should not sleep if there are no unfinished transactions', async () => { | ||
| // Stub methods if necessary |
There was a problem hiding this comment.
Why stub only in this test when all others, including the original test just set the stats directly ?
| const totalFinished = 10; | ||
| const totalSubmitted = totalFinished + unfinished; | ||
|
|
||
| sinon.stub(txnStats, 'getTotalSubmittedTx').returns(totalSubmitted); |
There was a problem hiding this comment.
again wonder why you stub here, but all others you set the txnStats values directly
|
|
||
| sinon.assert.calledOnce(sleepStub); | ||
| // Expected sleep time should be 4 * backOffTime = 400ms | ||
| sinon.assert.calledWith(sleepStub, 4 * controller.backOffTime); |
There was a problem hiding this comment.
This is 1 more than the zeroSuccessfulCounter, would be good to show this in this test
Checklist
Issue/User story
Partially addresses #1606
Steps to Reproduce
Existing issues
Design of the fix
Validation of the fix
After this PR: Running the tests in caliper-core the listed %stmts in the code coverage report should tally with the below listed
Before the PR: Running the tests in caliper-core the listed %stmts in the code coverage report the below listed was what was there before
What documentation has been provided for this pull request