Skip to content
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

Add tests for rate-control/compositeRate.js #1572

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 149 additions & 0 deletions packages/caliper-core/test/worker/rate-control/compositeRate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

const chai = require('chai');
const sinon = require('sinon');
const expect = chai.expect;
const {createRateController} = require('../../../lib/worker/rate-control/compositeRate.js');
const TransactionStatisticsCollector = require('../../../lib/common/core/transaction-statistics-collector');
const TestMessage = require('../../../lib/common/messages/testMessage.js');

describe('CompositeRateController', () => {
let testMessage;
let CompositeRateController;
let stats;
let clock;
beforeEach(() => {
let msgContent = {
label: 'query2',
rateControl: {
type: 'composite-rate',
opts: {
weights: [1],
rateControllers: [{ type: 'fixed-rate', opts: {} }]
},
},
workload: {
module: './../queryByChannel.js',
},
testRound: 0,
txDuration: 250,
totalWorkers: 2,
workerArgs: 0,
numb: 0,
};
testMessage = new TestMessage('test', [], msgContent);
stats = new TransactionStatisticsCollector(0, 0, 'query2');
CompositeRateController = createRateController(testMessage, stats, 0);
});

describe('#constructor', () => {
it('should correctly initialize with default settings', () => {
expect(CompositeRateController.activeControllerIndex).to.equal(0);
expect(CompositeRateController.controllers.length).to.equal(1);
});
});

describe('applyRateControl', () => {
Sweetdevil144 marked this conversation as resolved.
Show resolved Hide resolved
it('should apply rate control correctly', async () => {
expect(() => CompositeRateController.applyRateControl()).be.a(
'function'
);
});
});

describe('#_prepareControllers', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to be directly testing internal methods (ie ones that start with _). All tests should drive the public apis.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we incorporate _prepareControllers within applyRateControl?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessarily about incorporating especially if the internal method is called from multiple public methods. You need to write tests that make sense for the public methods and if the internal method gets called multiple times by the tests then that's fine. As an example a public method which has multiple code flows but to test all those code flows you may end up always calling the private method which is expected.

it('should throw error when weights and rateControllers are not arrays', async () => {
testMessage.content.rateControl.opts.weights = 'not an array';
testMessage.content.rateControl.opts.rateControllers = 'not an array';
expect(() => createRateController(testMessage, stats, 0)).to.throw(
'Weight and controller definitions must be arrays.'
);
});

it('should throw error when weights and rateControllers lengths are not the same', async () => {
testMessage.content.rateControl.opts.weights = [1, 2];
testMessage.content.rateControl.opts.rateControllers = ['composite-rate'];
expect(() => createRateController(testMessage, stats, 0)).to.throw(
'The number of weights and controllers must be the same.'
);
});

it('should throw error when weights contains non-numeric value', async () => {
testMessage.content.rateControl.opts.weights = [1, 'not a number'];
testMessage.content.rateControl.opts.rateControllers = [
'composite-rate',
'composite-rate',
];
expect(() => createRateController(testMessage, stats, 0)).to.throw(
'Not-a-number element among weights: not a number'
);
});

it('should throw error when weights contains negative number', async () => {
testMessage.content.rateControl.opts.weights = [1, -2];
testMessage.content.rateControl.opts.rateControllers = [
'composite-rate',
'composite-rate',
];
expect(() => createRateController(testMessage, stats, 0)).to.throw(
'Negative element among weights: -2'
);
});

it('should throw error when all weights are zero', async () => {
testMessage.content.rateControl.opts.weights = [0, 0];
testMessage.content.rateControl.opts.rateControllers = [
'composite-rate',
'composite-rate',
];
expect(() => createRateController(testMessage, stats, 0)).to.throw(
'Every weight is zero.'
);
});
});

describe('#_controllerSwitchForDuration', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to be directly testing internal methods (ie ones that start with _). All tests should drive the public apis.

beforeEach(() => {
CompositeRateController.controllers = [
{ isLast: false, relFinishTime: 100, txStatSubCollector: { deactivate: () => {}, activate: () => {} }, controller: { end: async () => {} } },
{ isLast: true, relFinishTime: 200, txStatSubCollector: { deactivate: () => {}, activate: () => {} }, controller: { end: async () => {} } }
];
CompositeRateController.activeControllerIndex = 0;
CompositeRateController.stats = {
getRoundStartTime: () => 1000,
getTotalSubmittedTx: () => 10,
};
stats = new TransactionStatisticsCollector(0, 0, 'query2');
clock = sinon.useFakeTimers();
});

afterEach(() => {
clock.restore();
});
it('should not switch if current controller is last', async () => {
CompositeRateController.activeControllerIndex = 1;
await CompositeRateController._controllerSwitchForDuration();
expect(CompositeRateController.activeControllerIndex).to.equal(1);
});

it('should not switch if it is not time yet', async () => {
global.Date.now = () => 100;
await CompositeRateController._controllerSwitchForDuration();
expect(CompositeRateController.activeControllerIndex).to.equal(0);
});
});
});