Skip to content

add more tests for CaliperEngine#1706

Open
tunedev wants to merge 1 commit into
hyperledger-caliper:mainfrom
tunedev:add-more-tests-to-caliper-engine
Open

add more tests for CaliperEngine#1706
tunedev wants to merge 1 commit into
hyperledger-caliper:mainfrom
tunedev:add-more-tests-to-caliper-engine

Conversation

@tunedev

@tunedev tunedev commented Aug 29, 2025

Copy link
Copy Markdown
Contributor

Checklist

  • 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

Partially addresses #1557

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

  • caliper-core/lib/manager/caliper-engine.js: 100

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

  • caliper-core/lib/manager/caliper-engine.js: 65.11

What documentation has been provided for this pull request

@ryjones
ryjones requested a review from a team August 29, 2025 19:36

@davidkel davidkel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tunedev Only a minor issue about how we setup the flow options and then just need to set the specific flow option to true that is being tested, plus cleanup of the whitespace. But all good apart from that, thanks for the PR (sorry it took so long to review)

stubbedCaliperUtils.getFlowOptions.reset();
stubbedCaliperUtils.execAsync.reset();

stubbedCaliperUtils.getFlowOptions.returns({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure why this is here and probably can be removed

let mockAdapter;

beforeEach(() => {
benchSandbox = sinon.createSandbox();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it may be good here to set all the flows to false, then for each of the tests ensure that only the flows being tested get set.

const testCMD = 'echo "Starting network"'

stubbedCaliperUtils.getFlowOptions.returns({
performStart: true,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's just set the flow option that is going to be tested here to true, and not include the others


it('should handle errors during start command execution', function() {
// TODO: Implement test
it('should handle errors during start command execution', async function() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should explicitly set the flow under test here

});

it('should handle a non-string start command', async function() {
const networkConfig = { caliper: { command: { start: 1234 } } }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should explicitly set the flow under test here

});

const engine = new CaliperEngine({}, {}, () => {throw new Error('Failed to create adapter')});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can remove blank line

performInit: false,
performInstall: false,
performTest: false,
performEnd: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same comment as before

stubbedConfigUtils.get.withArgs('caliper-workspace').returns('./');
stubbedCaliperUtils.execAsync.resolves();
const engine = new CaliperEngine({}, networkConfig, () => mockAdapter);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can remove blank line

performInit: false,
performInstall: false,
performTest: false,
performEnd: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same comment as before

Comment thread packages/caliper-core/test/manager/caliper-engine.js Outdated
@tunedev
tunedev force-pushed the add-more-tests-to-caliper-engine branch from 8637b09 to bd1c2e7 Compare December 2, 2025 10:21
Signed-off-by: Babatunde Sanusi <swisskid95@gmail.com>
@tunedev
tunedev force-pushed the add-more-tests-to-caliper-engine branch from bd1c2e7 to ad0c2d5 Compare December 2, 2025 10:25
@tunedev
tunedev requested a review from davidkel December 2, 2025 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants