jasmine / jasmine-npm

A jasmine runner for node projects.
MIT License
376 stars 145 forks source link

Cannot specify failSpecWithNoExpectations via loadConfig #156

Closed coyoteecd closed 4 years ago

coyoteecd commented 4 years ago

I tried to use failSpecWithNoExpectations setting added in https://github.com/jasmine/jasmine/pull/1743. I cannot get it to work, due to - I believe - a bug in loadConfig() implementation.

My setup is using jasmine 3.5 in a nodejs environment; I have followed the advanced configuration guide from https://jasmine.github.io/setup/nodejs.html#load-configuration-from-a-file-or-from-an-object, specifically the loadConfig() method:

const runner = new Jasmine({});
runner.loadConfig({
    spec_dir: "./out-tsc/src",
    spec_files: [
      "**/*[sS]pec.js"
    ],
    stopSpecOnExpectationFailure: false,
    failSpecWithNoExpectations: true,
    random: true
  });

Test:

describe('Example', () => {
  it('should fail', () => {
  });
});

I expected the tests would fail, however they're successful.

Reason is that loadConfig only seems to take into account a predefined set of properties, ignoring everything else (instead of passing them as-is to env.configure():

Jasmine.prototype.loadConfig = function(config) {
  this.specDir = config.spec_dir || this.specDir;

  var configuration = {};

  if (config.stopSpecOnExpectationFailure !== undefined) {
    configuration.oneFailurePerSpec = config.stopSpecOnExpectationFailure;
  }

  if (config.stopOnSpecFailure !== undefined) {
    configuration.failFast = config.stopOnSpecFailure;
  }

  if (config.random !== undefined) {
    configuration.random = config.random;
  }

  if (Object.keys(configuration).length > 0) {
    this.env.configure(configuration);
  }

Notice that config is only expected to have stopSpecOnExpectationFailure, stopOnSpecFailure and random, nothing else - so my property is simply ignored.

Workaround

This solves the problem:

jasmine.getEnv().configure({
    failSpecWithNoExpectations: true
  });

... however it's ugly (I use TypeScript and jasmine.Env does not expose configure externally). If this was the expected usage, then what's the point of having loadConfig at all.

How to fix it

Either add the mapping code above the env.configure() call, so:

if (config.failSpecWithNoExpectations !== undefined) {
  configuration.failSpecWithNoExpectations = config.failSpecWithNoExpectations;
}

... or maybe reconsider the design and just copy all other keys from config to configuration, because the current approach means you're going to require updates for every new configuration setting being added to jasmine-core.

coyoteecd commented 4 years ago

Somewhat related: the console reporter should also be updated to write some info in the error summary, specifically in printFailedExpectations function. What happens now is that the spec name is written, but no stack trace and no additional information, it looks like this:

Randomized with seed 29344
Started
F.

Failures:
1) Example should fail

2 specs, 1 failure
Finished in 0.009 seconds
slackersoft commented 4 years ago

You're right. It looks like we missed a couple of bits when that feature got added. I would be happy to review a pull request here to correctly pass the configuration option through and print out the error(s) correctly.

Thanks for using Jasmine!

Masterxilo commented 4 years ago

I can confirm that this also means that the feature currently cannot be configured at all in spec/support/jasmine.json (the config file). I put

"failSpecWithNoExpectations": true

but the tests containing no expectations still work.

The requirements for new features should IMO include the production of a complete project showcasing the feature, with all ways to configure it shown to be working properly to catch such things in the future...

Masterxilo commented 4 years ago

Another problem is this: When a test finally does fail because it has no expectations, there is no error message indicating that this is the cause of the failure.

coyoteecd commented 4 years ago

@Masterxilo The package uses the same loadConfig function to process config options in jasmine.json, so that's why it doesn't work. I just submitted a pull request with a fix for it, that also writes a message when using the ConsoleReporter.

coyoteecd commented 4 years ago

@sgravrock @slackersoft Mind releasing to npm as well?

Masterxilo commented 4 years ago

This issue has been closed, but the latest version available on npm still has the problem...

coyoteecd commented 4 years ago

@sgravrock @slackersoft it's been half a year already with no release :(

sgravrock commented 4 years ago

We've just released 3.6, which includes this change. Enjoy.