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

map test: reports partial result after an error (series & parallel) #16

Merged

Conversation

brodybits
Copy link
Contributor

These test cases cover the effects of a for loop in initializeResults(), in lib/helpers.js.

Without this kind of a test case, it would be possible to remove the for loop (and an internal variable) without failing the test suite:

diff --git a/lib/helpers.js b/lib/helpers.js
index b0e913c..364c8c1 100644
--- a/lib/helpers.js
+++ b/lib/helpers.js
@@ -20,17 +20,8 @@ function defaultExtensions(extensions) {
 }
 
 function initializeResults(values) {
-  var keys = Object.keys(values);
   var results = Array.isArray(values) ? [] : {};
 
-  var idx = 0;
-  var length = keys.length;
-
-  for (idx = 0; idx < length; idx++) {
-    var key = keys[idx];
-    results[key] = undefined;
-  }
-
   return results;
 }

This is a code mutation that I discovered through mutation testing using Stryker Mutator, which I will propose in an upcoming PR.

For the parallel map API call, the test case waits for both the callback and remaining iterator to finish. A little tricky, hopefully not too fragile:)

@phated phated added this to To do in v5 Apr 28, 2020
@phated phated moved this from To do to In progress in v5 Oct 21, 2020
@phated phated force-pushed the test-map-reports-partial-result-after-error branch from cf8a4cb to 96095dd Compare June 25, 2022 00:59
Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Thanks for this! I'm sorry it took so long to get to, but we are doing a pass at all the gulpjs modules.

I've rebased on the new CI and everything looks good. Cheers 🍻

@phated phated merged commit 454a9e8 into gulpjs:master Jun 25, 2022
@phated phated moved this from In progress to Done in v5 Jun 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v5
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants