gulp-jasmine icon indicating copy to clipboard operation
gulp-jasmine copied to clipboard

Upgrade to jasmine 4

Open mjeanroy opened this issue 3 years ago • 3 comments

Hi @sindresorhus,

Would you be interested in a PR to migrate to Jasmine 4? If you're ok, I can work on this and submit a PR, otherwise no worries 😉

Thanks!

mjeanroy avatar Feb 13 '22 13:02 mjeanroy

👍

sindresorhus avatar Feb 13 '22 13:02 sindresorhus

The issues, explained

I spent several hours yesterday taking a look at the updates in this branch and how the gulp-jasmine tests interact with the Jasmine 4 engine. Here's what I found:

  1. For performance reasons, Jasmine engine is using a Singleton pattern for its environment. It doesn't matter how many times you call new Jasmine(options), you will get the same instance unless you force a shutdown in between.

  2. By setting jasmine.exitOnCompletion = false, you are disabling the shutdown. (A premature shutdown can prevent promises from completing, so it's an engineering workaround on their end.)

  3. Each spec file is represented as a "suite" of tests internally. Duplicate files are skipped to avoid duplication of effort, resulting in overall status of "incomplete" from the engine. (IMO: "skipped" might have been a better choice.) You know this has happened when you get the following verbose output.

    Running 0 specs. 
    0 specs, 0 failures. 
    Finished in 0 seconds.
    
  4. And here's the gut punch: The Jasmine engine returns one "overall status" for all suites combined.

    • This happens asynchronously, no matter which spec file you think you are running. So, even if the current spec passes all tests, the engine might still return an overall result of "failed."
    • The information returned from execute does not tell you which suite caused the failure, so you'd have to crack open the engine to map failures correctly.

I know the Jasmine folks have deprecated engine shutdown, but without a granular way to determine which part of the suite is reporting errors, this seems premature IMO. (This might have changed in Jasmine 5 alpha, more investigation needed.)

Why this matters

None of these changes would matter if all the gulp-jasmine tests were designed to pass, but this is not the case. The fail-fixture.js is specifically designed to test what happens when tests fail. (Excellent choice, BTW.)

NOTE These issues are not likely to impact regular users of gulp-jasmine as test failures generally should be reported.

  1. The ava tests are assuming that each fixture is run in isolation. (False.)
  2. Only the failure tests are configured to ignore errors. (Failure are reported across the entire suite.)
  3. Files are submitted multiple times to test different features. (Duplicates are skipped.)

Some architectural suggestions

In order to run a comprehensive suite of tests for gulp-jasmine, you might consider the following suggestions.

  • Allow the caller to customize the default "exitOnCompletion = false" setting, making it possible to require a shutdown between invocations of the gulp-jasmine pipeline. Then, enforce a shutdown between ava tests.

  • Write a second ava suite to run the fail-fixture.js tests separately from those that succeed. Manually restart Jasmine engine between these two ava suites.

A note about promise rejection

Finally, just a reminder that await will throw when a promise is rejected for any reason. The code in index.js is wrapped in a try/catch, but the catch handler turns around and sends a PluginError back to the flush callback, causing a secondary "unhandled exception" in the transform pipeline. This might be the desired effect for production pipelines that need to crash when a test fails, but it begs the question how to detect these edge cases more gracefully in the ava tests.

  • Add setting to configure pipeline to return 'jasmineError' events rather than PluginError exceptions. This would allow the pipeline owner to more gracefully detect and respond to pipeline failures caused by rejected promises.

jsuddsjr avatar Apr 26 '23 17:04 jsuddsjr

If anyone wants to work on this, see the initial attemp in https://github.com/sindresorhus/gulp-jasmine/pull/106

sindresorhus avatar Jun 20 '23 12:06 sindresorhus