Add tests for `rate-control/compositeRate.js`

Sweetdevil144 commented 4 months ago

This PR aims to create unit-tests for compositeRate.js class within our caliper-core package.

However, the current tests are failing due to following reasons.

Within our packages/caliper-core/lib/worker/rate-control/compositeRate.js, we find the following line :

https://github.com/hyperledger/caliper/blob/21a98f496c850840c211a670c32fcfa9240612bb/packages/caliper-core/lib/worker/rate-control/compositeRate.js#L76

However, I tried logging out this.options and was returned with an empty Object as output.

console.log('Options: ', this.options);
let weights = this.options.weights;
let rateControllers = this.options.rateControllers;

I was returned with following logs :

Options:  {}
2024.05.12-00:43:27.856 error [caliper] [composite-rate-controller]     Weight and controller definitions must be arrays.
      2) should throw error when weights and rateControllers lengths are not the same
(Similar errors for other tests too)

Due to this our initial check at line 86, that is :

https://github.com/hyperledger/caliper/blob/21a98f496c850840c211a670c32fcfa9240612bb/packages/caliper-core/lib/worker/rate-control/compositeRate.js#L79

Always throws an error. However, if we replace the line 76 above with

let weights = this.testMessage.content.weights;

Then, our tests pass.

What would be the most optimum fix for such cases?

Checklist

[x] A link to the issue/user story that the pull request relates to
[ ] How to recreate the problem without the fix
[ ] Design of the fix
[ ] How to prove that the fix works
[ ] Automated tests that prove the fix keeps on working
[ ] Documentation - any JSDoc, website, or Stackoverflow answers?

Issue/User story

Steps to Reproduce

1. 2. 3. 4.

Existing issues

This issue is a small part of #1557 which aims to increase overall code-coverage for the caliper-core package.

Sweetdevil144 commented 4 months ago

Considering above changes have been applied for proper data-handling within _prepareControllers, we get a new error causing recursive functional callings as follow :

 at CompositeRateController._prepareControllers (lib/worker/rate-control/compositeRate.js:173:37)
      at new CompositeRateController (lib/worker/rate-control/compositeRate.js:69:14)
      at createRateController (lib/worker/rate-control/compositeRate.js:275:12)
      at new RateControl (lib/worker/rate-control/rateControl.js:59:27)
      at CompositeRateController._prepareControllers (lib/worker/rate-control/compositeRate.js:173:37)
      at new CompositeRateController (lib/worker/rate-control/compositeRate.js:69:14)
      at createRateController (lib/worker/rate-control/compositeRate.js:275:12)
      at Context.<anonymous> (test/worker/rate-control/compositeRate.js:49:35)
      at process.processImmediate (node:internal/timers:478:21)

Sweetdevil144 commented 4 months ago

@davidkel can you help me with this in any way possible?

davidkel commented 4 months ago

@Sweetdevil144 Unfortunately your change is not correct. The reason you are getting an error and options is an empty object is because that's what you have set it to in your test when you created a mock message.

            rateControl: {
                type: 'composite-rate',
                opts: {},

Sweetdevil144 commented 4 months ago

@Sweetdevil144 Unfortunately your change is not correct. The reason you are getting an error and options is an empty object is because that's what you have set it to in your test when you created a mock message.
            rateControl: {
                type: 'composite-rate',
                opts: {},

Thanks. The test cases work fine now. However, I found a bug within our packages/caliper-core/lib/worker/rate-control/compositeRate.js. Can you confirm whether I'm correct or now?

I notices that we had been extracting Weights and rateControllers via following configurations :

        let weights = this.options.weights;
        let rateControllers = this.options.rateControllers;

Whereas, it should have been as below :

        let weights = this.testMessage.content.weights;
        let rateControllers = this.testMessage.content.rateControllers;

Can you confirm whether I'm correct or not @davidkel ? I'm pushing the overall applied changes for now. I'll refract them if you feel these aren't corresponding to any internal bug in system.

davidkel commented 4 months ago

@Sweetdevil144 Sorry, no your change is not correct. Not sure how you have come to this conclusion but if you look in the documentation https://hyperledger.github.io/caliper/v0.6.0/rate-controllers/#composite-rate you will see the format of the rate controller spec. weights and rateControllers are properties of opts and opts is extracted into this.options in the constructor see https://github.com/hyperledger/caliper/blob/56ecafa073ddb751375f3357200f84b4264ee13e/packages/caliper-core/lib/worker/rate-control/rateInterface.js#L33

Sweetdevil144 commented 4 months ago

@davidkel fixed. Can you review it?

hyperledger / caliper