qunitjs / js-reporters

📋 Common Reporter Interface (CRI) for JavaScript testing frameworks.
MIT License
60 stars 18 forks source link

Fix Mocha Adapter #50

Closed flore77 closed 8 years ago

flore77 commented 8 years ago

After the new testing we need to fix some issues in the adapter.

Issues

[1] The current adapter do not permit suites or tests with the same name, for example:

describe('suite1', function() {
  it('test1', function() {})
})

describe('suite2', function() {
  it('test1', function() {})
})

Both tests will have the same suite as suite parent, which is suite2.

This is happening because the tests are stored in an object by their title (description) which is non-unique, the same happens for suites.

[2] The objects that are emitted are then mutated. Emitted and mutated.

[3] The inner suite, in nested suites, has as name the concatenation of the names from the top outter suite to the most inner suite. This is done by us in our adapter.

[4] The tests on the startTest event, have also the properties runtime, status, errors, they are undefined, but maybe they shouldn't be there at all.

Solutions

[1] See [2] solves also this.

[2] Construct every time new objects, by calling the convert functions, without indexing the objects. When converting a test check if the Mocha object has an error property, if it has then we must build a test end, else a test start.

[3] Concatenate the suites names.

[4] Leave it like this, with the props to be undefined.

flore77 commented 8 years ago

@jzaefferer please review this.

jzaefferer commented 8 years ago

[1] I'm not sure what that would look like, let's give it a try.

[2] Let's try not mutating objects, instead creating them from scratch every time. So instead of grabbing the previous object, call the convert method again and add to the result.

[3] Looks like we didn't look into that when doing the research last year. Is it possible to construct the full name from the emitted data? If yes, then we don't need to do the concatenation, if no, we do. Besides we can still look at how frameworks deal with this, it could be convenient enough for reporters to have this provided to them.

[4] Leaving the value as undefined seems fine to me. The spec should be explicit about that though.

flore77 commented 8 years ago

Hmm, I guess the solution for [2], would also solve [1] :smile: I will start with this one.

flore77 commented 8 years ago

For [3] Mocha does not provide a full name, it is providing only the name of the test, so we must do the concatenation.

I have looked over karma-mocha and I have seen that they are also storing a full name, but not concatenated, instead putting every suite name in an array (I don't for what they need it), but how you have said, it would be useful that our adapter provide it already. So we do keep the concatenation @jzaefferer ?

jzaefferer commented 8 years ago

If we provide the full name, will be it possible to also access the individual pieces?

flore77 commented 8 years ago

No, because we are concatenating it as a string, so we will not know which is the name of the first, second, ..., suite. For our tests fixture: the nested test has as suiteName: outter suite inner suite, and you cannot tell in how many suites the test is nested and which are their names.

jzaefferer commented 8 years ago

Okay, let's stick with that.