Write tests by oliwialesniak-mate · Pull Request #88 · mate-academy/qa_array-method-reduce

oliwialesniak-mate · 2025-09-29T18:31:19Z

No description provided.

mateacademy-ai-mentor

Review Summary

I’m requesting changes because there are several critical blockers that violate explicit checklist requirements for the reduce tests.

Critical blockers to fix:
1. Add explicit assertions for the first-callback semantics. With initialValue, assert first call receives (acc === initialValue, curr === array[0], index === 0); without initialValue, assert first call receives (acc === array[0], curr === array[1], index === 1). Suggestion: capture callback calls in an array of tuples [acc, curr, index, src] and assert values and call counts. Apply to the summing tests at src/reduce.test.js lines ~15–28.
2. In the “provides index and array arguments” test (lines ~38–50), also record the accumulator and assert the callback was invoked exactly three times and that acc/curr match the expected sequence for the given input and initialValue.
3. Change the empty-array-without-initialValue assertion to expect a specific TypeError: expect(() => [].reduce2(cb)).toThrow(TypeError) (lines ~52–54).
4. For “returns the initial value on empty array with initial value” (lines ~56–59), add a spy/counter and assert the callback was not called at all.
5. Add the missing required test cases before the describe ends: single-element arrays (with/without initialValue), sparse array hole skipping and iteration order (assert visited indices and call counts), length-snapshot semantics when mutating during iteration (e.g., push; ensure new elements aren’t visited), ignoring prototype-inherited numeric properties, ensuring reduce does not mutate the input array, handling undefined/null elements as regular values, propagation of errors thrown inside the callback, and calling reduce on null/undefined receiver should throw TypeError.
Note: Ensure the implementation module import/export exists and matches the test (currently require('./reduce')), otherwise tests will fail at runtime with MODULE_NOT_FOUND.

You’ve built a solid base and the tests read well; adding these assertions and edge cases will bring the suite up to MDN-level semantics and the assignment checklist. Keep it up!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-09-29T18:36:06Z

src/reduce.test.js

+  it("sums an array with an initial value", () => {
+    const result = [1, 2, 3, 4].reduce2((acc, val) => acc + val, 0);
+    expect(result).toBe(10);
+  });
+
+  it("sums an array without an initial value", () => {
+    const result = [1, 2, 3, 4].reduce2((acc, val) => acc + val);
+    expect(result).toBe(10);
+  });


The summing tests check only the final result but do not assert the callback-argument semantics for the first invocation. Per the checklist you must verify: with initialValue the first callback is called with (accumulator === initialValue, currentValue === array[0], index === 0); without initialValue the first callback is called with (accumulator === array[0], currentValue === array[1], index === 1). Add assertions or use a spy to capture and assert these argument values and the expected number of invocations.

mateacademy-ai-mentor · 2025-09-29T18:36:06Z

src/reduce.test.js

+  it("provides index and array arguments to callback", () => {
+    const arr = [10, 20, 30];
+    const calls = [];
+    arr.reduce2((acc, val, idx, src) => {
+      calls.push([val, idx, src]);
+      return acc + val;
+    }, 0);
+
+    expect(calls[0][1]).toBe(0);
+    expect(calls[1][1]).toBe(1);
+    expect(calls[2][1]).toBe(2);
+    expect(calls[0][2]).toBe(arr);


The 'provides index and array arguments to callback' test records value, index and source but does not assert the accumulator/currentValue arguments or the total number of callback calls. Update this test to capture the accumulator too (e.g., push a 4-tuple) and assert that the callback was invoked exactly three times and that the accumulator/currentValue match expectations for each call.

mateacademy-ai-mentor · 2025-09-29T18:36:06Z

src/reduce.test.js

+  it("throws on empty array without initial value", () => {
+    expect(() => [].reduce2((acc, val) => acc + val)).toThrow();


This test asserts that an error is thrown for empty arrays without an initial value, but it uses a generic toThrow(). The specification requires a TypeError to be thrown. Change the assertion to expect a TypeError specifically, e.g., expect(() => [].reduce2(...)).toThrow(TypeError).

mateacademy-ai-mentor · 2025-09-29T18:36:06Z

src/reduce.test.js

+  it("returns the initial value on empty array with initial value", () => {
+    const result = [].reduce2((acc, val) => acc + val, 42);
+    expect(result).toBe(42);


The test 'returns the initial value on empty array with initial value' verifies the return value but does not assert that the callback was never called. Add a spy or counter to ensure the callback is not invoked when the array is empty and an initialValue is provided.

mateacademy-ai-mentor · 2025-09-29T18:36:07Z

src/reduce.test.js

+it("flattens an array of arrays", () => {
+  const result = [[1, 2], [3, 4], [5]].reduce2(
+    (acc, val) => acc.concat(val),


The flattening test is fine for result correctness, but consider also asserting the number of callback invocations and that the original array reference passed into callbacks equals the source array. This helps verify correct callback argument passing and invocation counts for nested-array scenarios.

mateacademy-ai-mentor · 2025-09-29T18:36:07Z

src/reduce.test.js

+    []
+  );


Several required behaviors from the task checklist are missing entirely: tests for single-element arrays (with/without initialValue), sparse-array hole-skipping and iteration order, length-snapshot semantics (mutating array during iteration), ignoring prototype-inherited numeric properties, ensuring reduce does not mutate the input array, handling undefined/null elements, propagation of errors thrown inside the callback, and calling reduce on null/undefined receiver. Add dedicated tests for each of these cases (append new it() blocks before the describe block ends).

mateacademy-ai-mentor

Review Summary

I’m requesting changes because there are a few checklist-critical blockers in the test suite that could either prevent the tests from running or leave key semantics unverified.

Critical issues to fix:

Line 1: Ensure the implementation file exists and exports a named reduce matching const { reduce } = require('./reduce'). If ./reduce.js is missing or exports a default, tests will fail at require-time.
Lines 59–61: For the single-element array without an initial value, assert the callback is not invoked. Use a spy (e.g., const cb = jest.fn(); expect([5].reduce2(cb)).toBe(5); expect(cb).not.toHaveBeenCalled();). This is a required native semantic.
Line 48: Add a test that passing a non-function as the callback throws TypeError (e.g., expect(() => [1,2].reduce2(null)).toThrow(TypeError) and similar). This is explicitly required.
Lines 13–24 and 27–38: When you capture callback args, assert the 4th arr param is strictly equal to the original array: expect(calls[0][3]).toBe(originalArray). This verifies the required callback signature/identity.
Lines 75–84: In the sparse-array test, also assert the original array still has the hole (e.g., expect(1 in arr).toBe(false)) and that the callback’s arr argument equals the original array.
Lines 87–95: Add explicit callback call-count assertions (e.g., expect(calls).toBe(2)) in the length-snapshot test to prove pushed elements aren’t visited.
Additionally, add an objects test (e.g., reducing an array of objects into a merged object) to cover object accumulator/element handling.

Minor improvement:

Revisit the prototype-inherited numeric property test to ensure the inherited index is actually within the traversed range; then clean up prototype changes in a finally block.

You’ve covered many tricky behaviors well—these targeted additions will bring the suite to full compliance and robustness. Keep up the solid work!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-09-29T18:44:03Z

src/reduce.test.js

@@ -1,8 +1,9 @@
 'use strict';


Line 1: The tests import reduce with const { reduce } = require('./reduce');. Ensure an implementation file exists at ./reduce.js (or index.js under ./reduce) and that it exports a named reduce (not a default). If the module is missing or the export shape differs, tests will fail at require-time.

mateacademy-ai-mentor · 2025-09-29T18:44:03Z

src/reduce.test.js

  });

-  it('should ', () => {
+  describe('basic behavior', () => {
+    it('sums with initial value, checks first callback args', () => {
+      const calls = [];
+      const result = [1, 2, 3].reduce2((acc, curr, idx, arr) => {
+        calls.push([acc, curr, idx, arr]);
+        return acc + curr;
+      }, 0);

+      expect(result).toBe(6);
+      expect(calls.length).toBe(3);


Lines 13–24: Good: you capture [acc, curr, idx, arr] in calls. Add an explicit assertion that the arr argument passed into the callback is strictly equal to the original array (e.g., store the original array in a variable and assert expect(calls[0][3]).toBe(original)), to satisfy the requirement that the callback's fourth argument is the original array object.

mateacademy-ai-mentor · 2025-09-29T18:44:03Z

src/reduce.test.js

+      expect(calls[0][2]).toBe(0);
+    });
+
+    it('sums without initial value, checks first callback args', () => {
+      const calls = [];
+      const result = [1, 2, 3].reduce2((acc, curr, idx, arr) => {
+        calls.push([acc, curr, idx, arr]);
+        return acc + curr;
+      });
+
+      expect(result).toBe(6);
+      expect(calls.length).toBe(2);


Lines 27–38: In the 'sums without initial value' test you record the callback args but do not assert the arr identity. Add expect(calls[0][3]).toBe(originalArray) (or similar) so the test verifies the original array is passed as the fourth parameter and that indices are correct.

mateacademy-ai-mentor · 2025-09-29T18:44:03Z

src/reduce.test.js

+  });
+
+  describe('edge cases', () => {


Lines 59–61: The single-element-without-initial test asserts the returned value is 5 but does not assert the callback was never called. The checklist requires asserting the callback is not invoked in this case — replace or augment with a spy: const cb = jest.fn(); expect([5].reduce2(cb)).toBe(5); expect(cb).not.toHaveBeenCalled();.

mateacademy-ai-mentor · 2025-09-29T18:44:03Z

src/reduce.test.js

+      const cb = jest.fn();
+      const result = [].reduce2(cb, 42);
+      expect(result).toBe(42);
+      expect(cb).not.toHaveBeenCalled();


Line 48: Add a test asserting that passing a non-function as the callback throws TypeError. Example to add near this area:
it('throws TypeError when callback is not a function', () => { expect(() => [1,2].reduce2(null)).toThrow(TypeError); expect(() => [1,2].reduce2(123)).toThrow(TypeError); });

mateacademy-ai-mentor · 2025-09-29T18:44:03Z

src/reduce.test.js

+      expect(calls[0]).toEqual([10, 5, 0]);
+    });
+
+    it('skips undefined elements (instead of sparse array)', () => {
+      const arr = [1, undefined, 3];
+      const calls = [];
+      const result = arr.reduce2((acc, val, idx) => {
+        calls.push([val, idx]);
+        return acc + (val || 0);
+      }, 0);


Lines 75–84: Sparse-array test correctly asserts that holes are skipped. To fully satisfy the checklist, also assert the original array still has the hole (for example expect(1 in arr).toBe(false) or expect(Object.prototype.hasOwnProperty.call(arr, 1)).toBe(false)) and assert the arr argument passed to the callback equals the original array to show callbacks receive the original (hole-containing) array.

mateacademy-ai-mentor · 2025-09-29T18:44:04Z

src/reduce.test.js

+      expect(calls).toEqual([[1, 0], [undefined, 1], [3, 2]]);
+    });
+
+    it('uses length snapshot, ignores pushed values', () => {
+      const arr = [1, 2];
+      const result = arr.reduce2((acc, val, idx, src) => {
+        if (idx === 0) src.push(99);
+        return acc + val;
+      }, 0);


Lines 87–95: The length-snapshot test checks the numeric result and array mutation. Add an explicit assertion of how many times the callback was called (e.g., let calls=0; ... calls++ and expect(calls).toBe(2)) to prove newly pushed elements were not visited. This satisfies the checklist item requiring callback invocation counts when initialValue is present.

mateacademy-ai-mentor · 2025-09-29T18:44:04Z

src/reduce.test.js

+      expect(() => [].reduce2((a, b) => a + b)).toThrow(TypeError);
+    });
+


Lines 52–54: You test string concatenation which covers primitives. Add a test demonstrating object handling (e.g., reduce an array of objects into a single merged object or building a lookup) to satisfy the requirement to verify different data types (objects) as accumulator/elements.

mateacademy-ai-mentor · 2025-09-29T18:44:04Z

src/reduce.test.js

+    it('propagates errors thrown in callback', () => {
+      expect(() =>
+        [1, 2].reduce2(() => {
+          throw new Error('boom');
+        }, 0)
+      ).toThrow('boom');
+    });
+
+    it('throws TypeError when called on null/undefined', () => {
+      const cb = (a, b) => a + b;
+      expect(() => reduce(null, cb, 0)).toThrow(TypeError);
+      expect(() => reduce(undefined, cb, 0)).toThrow(TypeError);
+    });


Lines 121–133: Good: you assert that errors thrown in the callback propagate and that calling the raw reduce with null/undefined receiver throws TypeError. Keep these. Just ensure the reduce implementation exists and matches the call signatures used here (see comment at line 1).

mateacademy-ai-mentor

Review Summary

Educational approval: moving you forward while noting a few improvements to solidify reduce semantics.

What you did well:

Strong coverage of core behaviors: with/without initial value, empty-array semantics, non-mutation, error propagation, object accumulator, and property-based parity with native reduce.
Clear callback-argument checks and thoughtful edge cases.

What to improve (specific, actionable):

Ensure the import/export matches the test’s usage at src/reduce.test.js:1. The test does const { reduce } = require('./reduce'); verify src/reduce.js exists and exports a named reduce (e.g., module.exports = { reduce }). Otherwise the suite will fail at require-time.
Add a sparse-array holes test (edge-cases block). For example:
- const arr = [1, , 3]; const calls = []; arr.reduce2((acc, v, i) => { calls.push(i); return acc + (v || 0); }, 0); expect(calls).toEqual([0, 2]); This verifies missing indices are skipped per MDN.
Add a test for “one present element among holes” (no initial). Example: const arr = Array(3); arr[2] = 5; const cb = jest.fn(); const result = arr.reduce2(cb); expect(result).toBe(5); expect(cb).not.toHaveBeenCalled();
Fix null/undefined receiver TypeError assertions near the TypeError tests: if reduce is a method using this, use expect(() => reduce.call(null, cb, 0)).toThrow(TypeError) and .call(undefined, ...). If it’s a standalone function, pass an array as the first argument consistently.
Minor: where you intend to assert the 4th callback arg is the same array, prefer toBe(originalArray) over deep equality with a literal.

Great work overall—closing these gaps will make your test suite fully spec-aligned and more robust. Keep it up!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2025-09-29T18:56:21Z

src/reduce.test.js

@@ -1,8 +1,9 @@
 'use strict';

+const fc = require('fast-check');


The test imports the function via const { reduce } = require('./reduce'). Make sure ./reduce.js exists and exports a named reduce (for example module.exports = { reduce }). If the module exports a default or uses a different name/signature the test will fail at require-time.

mateacademy-ai-mentor · 2025-09-29T18:56:22Z

src/reduce.test.js

+          (acc, val, idx, arr) => {
+            calls.push([acc, val, idx, arr]);
+            return acc + val;
+          },


Add a test asserting that reduce skips holes (missing indices) in sparse arrays. Currently you test undefined as a present element but not the difference between a hole and an undefined value. Example to add in the edge-cases block: const arr = [1, , 3]; const calls = []; arr.reduce2((acc, v, i) => { calls.push(i); return acc + (v || 0); }, 0); expect(calls).toEqual([0,2]); — this checks that the middle hole was not visited.

mateacademy-ai-mentor · 2025-09-29T18:56:22Z

src/reduce.test.js

+      );
+      expect(result).toEqual({ x: 1, y: 2, z: 3 });
+    });
  });


Add a test for an array that contains only one present element among holes (e.g., const arr = Array(3); arr[2] = 5;). When called without an initial value reduce must return the single present element and must not call the callback. The current single-element tests cover dense arrays like [5], but not the sparse-only-present-element case.

mateacademy-ai-mentor · 2025-09-29T18:56:22Z

src/reduce.test.js

+      );
+      expect(result).toEqual([undefined, null, 1]);
+    });
+


The TypeError assertions for null/undefined currently call reduce(null, cb, 0) and reduce(undefined, cb, 0). If reduce is intended to be a method that uses this (you attach it to Array.prototype.reduce2 above), change these to expect(() => reduce.call(null, cb, 0)).toThrow(TypeError) and expect(() => reduce.call(undefined, cb, 0)).toThrow(TypeError) so the test actually tries to call the method with a null/undefined receiver. If reduce is a standalone function that expects the array as the first parameter, update the test to pass an array argument instead. Make the signature usage consistent.

oliwialesniak-mate added 2 commits September 29, 2025 20:30

Write tests

e2e56b8

Write tests

0992279

mateacademy-ai-mentor suggested changes Sep 29, 2025

View reviewed changes

Write tests

32fc7ab

oliwialesniak-mate requested a review from mateacademy-ai-mentor September 29, 2025 18:38

Write tests

dc32970

mateacademy-ai-mentor suggested changes Sep 29, 2025

View reviewed changes

oliwialesniak-mate added 2 commits September 29, 2025 20:45

Write tests

a60b5e8

Write tests

e4b075f

oliwialesniak-mate requested a review from mateacademy-ai-mentor September 29, 2025 18:50

Write tests

2099efc

mateacademy-ai-mentor approved these changes Sep 29, 2025

View reviewed changes

Write tests

dc8bdfc

		it("throws on empty array without initial value", () => {
		expect(() => [].reduce2((acc, val) => acc + val)).toThrow();

		expect(() => [].reduce2((a, b) => a + b)).toThrow(TypeError);
		});

		@@ -1,8 +1,9 @@
		'use strict';

		const fc = require('fast-check');

Conversation

oliwialesniak-mate commented Sep 29, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants