Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
10 changes: 5 additions & 5 deletions src/ifElse.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
'use strict';

/**
* @param condition
* @param first
* @param second
* @param {Function} condition
* @param {Function} first
* @param {Function} second
*/
function ifElse(condition, first, second) {
if (condition() === true) {
if (condition()) {
first();
} else {
second();
}
}

module.exports = { ifElse };
module.exports = ifElse;

Choose a reason for hiding this comment

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

Export mismatch (blocking): The tests import using destructuring: const { ifElse } = require('./ifElse'); which expects the module to export an object with an ifElse property. Here you export the function directly (module.exports = ifElse;), so ifElse will be undefined in the tests. Update the export to a named export object, for example:

module.exports = { ifElse };

or change the tests to const ifElse = require('./ifElse'); so the import matches the current export. This must be fixed for the tests to run.

41 changes: 38 additions & 3 deletions src/ifElse.test.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,46 @@
'use strict';

const { ifElse } = require('./ifElse');

Choose a reason for hiding this comment

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

Import/export mismatch: const { ifElse } = require('./ifElse'); expects a named export (an object with an ifElse property). The implementation exports the function directly (module.exports = ifElse), so this destructuring will result in ifElse being undefined and the tests will fail to run. Fix either by changing the import to const ifElse = require('./ifElse'); or by exporting an object from the implementation (module.exports = { ifElse };).

Choose a reason for hiding this comment

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

Import line uses destructuring (const { ifElse } = require('./ifElse');) which matches the current implementation export module.exports = { ifElse } in src/ifElse.js. No change needed to the import/export shape.


describe('ifElse', () => {
// const { ifElse } = require('./ifElse');
let firstMock;
let secondMock;
let conditionTrue;
let conditionFalse;

beforeEach(() => {
firstMock = jest.fn();
secondMock = jest.fn();
conditionTrue = () => true;

Choose a reason for hiding this comment

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

Make conditionTrue a mock so you can assert it was called exactly once and with no arguments. The task requires verifying the condition is invoked exactly once and without arguments; plain functions (() => true) cannot be inspected by Jest for call counts/args. Change to something like: conditionTrue = jest.fn(() => true);

conditionFalse = () => false;

Choose a reason for hiding this comment

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

Make conditionFalse a mock for the same reason as above. Change to something like: conditionFalse = jest.fn(() => false); so tests can assert it was called exactly once and with no args in the false case.

Choose a reason for hiding this comment

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

In beforeEach you set conditionTrue and conditionFalse to plain functions. The checklist requires using Jest mocks/spies so you can assert call counts and arguments. Replace these with mocks, for example:
conditionTrue = jest.fn(() => true) and conditionFalse = jest.fn(() => false).
This change lets you assert toHaveBeenCalledTimes(1) and toHaveBeenCalledWith() for the condition (required behavior).

});

it('should call first callback if condition returns true', () => {
ifElse(conditionTrue, firstMock, secondMock);

Choose a reason for hiding this comment

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

In the "should call first callback if condition returns true" test you should assert the condition was called exactly once and with no arguments (requirement: "condition is invoked exactly once and without arguments"). After converting conditionTrue to a mock, add expect(conditionTrue).toHaveBeenCalledTimes(1); and expect(conditionTrue).toHaveBeenCalledWith(); before asserting which branch ran.


expect(firstMock).toHaveBeenCalled();

Choose a reason for hiding this comment

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

Update these assertions to check exact call count and that firstMock was called without arguments. The task requires first be called exactly once and with no arguments when condition returns true. Replace expect(firstMock).toHaveBeenCalled(); with:

  • expect(firstMock).toHaveBeenCalledTimes(1);
  • expect(firstMock).toHaveBeenCalledWith();
    Keep the assertion that secondMock was not called (or use toHaveBeenCalledTimes(0)).

expect(secondMock).not.toHaveBeenCalled();

Choose a reason for hiding this comment

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

The 'should call first callback if condition returns true' test only checks that firstMock was called and secondMock was not. Per the checklist you must also assert that the condition mock was called exactly once and with no arguments, and that firstMock was called exactly once and with no arguments. Replace loose assertions with precise ones, e.g.:
expect(conditionTrue).toHaveBeenCalledTimes(1)
expect(conditionTrue).toHaveBeenCalledWith()
expect(firstMock).toHaveBeenCalledTimes(1)
expect(firstMock).toHaveBeenCalledWith()
Keep expect(secondMock).not.toHaveBeenCalled() to ensure the other callback was not invoked.

});

it('should ', () => {
it('should call second callback if condition returns false', () => {
ifElse(conditionFalse, firstMock, secondMock);

Choose a reason for hiding this comment

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

In the "should call second callback if condition returns false" test you likewise need to assert the condition was called exactly once and with no args. After converting conditionFalse to a mock, add expect(conditionFalse).toHaveBeenCalledTimes(1); and expect(conditionFalse).toHaveBeenCalledWith();.


expect(secondMock).toHaveBeenCalled();

Choose a reason for hiding this comment

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

Update the assertions for the false case to verify secondMock was called exactly once and without arguments. Replace expect(secondMock).toHaveBeenCalled(); with:

  • expect(secondMock).toHaveBeenCalledTimes(1);
  • expect(secondMock).toHaveBeenCalledWith();
    And keep/replace expect(firstMock).not.toHaveBeenCalled(); with expect(firstMock).toHaveBeenCalledTimes(0); if you want an explicit count assertion.

expect(firstMock).not.toHaveBeenCalled();

Choose a reason for hiding this comment

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

The 'should call second callback if condition returns false' test also uses loose assertions. Make the same upgrades as above: ensure the condition mock was called exactly once with no args, ensure secondMock was called exactly once with no args, and assert firstMock was not called. Use toHaveBeenCalledTimes(1) and toHaveBeenCalledWith() for the positive checks.

});

// write tests here
it('should work when condition is a dynamic function', () => {
const dynamicCondition = jest.fn(() => true);
ifElse(dynamicCondition, firstMock, secondMock);

expect(dynamicCondition).toHaveBeenCalled();

Choose a reason for hiding this comment

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

Dynamic condition test: you already use jest.fn(() => true) for dynamicCondition which is good, but the assertions are loose. Assert the condition was called exactly once and with no args, and assert firstMock was called exactly once and with no args. Replace toHaveBeenCalled() calls with toHaveBeenCalledTimes(1) and add toHaveBeenCalledWith() where appropriate. Also ensure secondMock is asserted as not called (not.toHaveBeenCalled() or toHaveBeenCalledTimes(0)).

expect(firstMock).toHaveBeenCalled();
expect(secondMock).not.toHaveBeenCalled();

Choose a reason for hiding this comment

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

The dynamic condition test correctly uses jest.fn() for dynamicCondition, which is good. However its assertions are loose. Change them to exact checks:
expect(dynamicCondition).toHaveBeenCalledTimes(1) and expect(dynamicCondition).toHaveBeenCalledWith();
expect(firstMock).toHaveBeenCalledTimes(1) and expect(firstMock).toHaveBeenCalledWith();
and keep expect(secondMock).not.toHaveBeenCalled().
These confirm exactly one invocation and that no arguments were passed (checklist).

});

it('should not return any value', () => {
const result = ifElse(conditionTrue, firstMock, secondMock);
expect(result).toBeUndefined();

Choose a reason for hiding this comment

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

The test checking that the function returns no value (expect(result).toBeUndefined();) already addresses the requirement that ifElse does not return a value. Keep this assertion as-is.

Comment on lines +57 to +59

Choose a reason for hiding this comment

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

The 'should not return any value' test asserts the return is undefined which meets the return requirement. To make this test stronger, call ifElse with a mocked condition (e.g. jest.fn(() => true)) and also assert the condition was called exactly once. Right now conditionTrue is a plain function (see beforeEach); if you change it to a mock as suggested, this test will also be able to assert the condition call count.

});
});

Loading