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

fix(mocha framework): Select tests based on name #413

Merged
merged 5 commits into from
Oct 20, 2017

Conversation

nicojs
Copy link
Member

@nicojs nicojs commented Oct 9, 2017

Select tests in the MochaTestFramework based on it's name, rather than the order in which it is added to a test suite.

  • Change TestFramework interface to now filter based on test id and name.
  • Add TestSelection which represents a selected test using id and name.
  • Update JasmineTestFramework to now use the new TestSelection format (still based on ID).
  • Update MochaTestFramework to use the title to select tests.
  • Add integration tests to both JasmineTestFramework and MochaTestFramework to prevent this sort of thing from happening in the future.

Fixes #249

BREAKING CHANGES:

  • Change api of TestFramework. It now provides an array of TestSelection objects, instead of an array of numbers with test ids.

Select tests in the `MochaTestFramework` based on it's name, rather than the order in which it is added to a test suite.

* Change TestFramework interface to now filter based on test `id` and `name`.
* Add `TestSelection` which represents a selected test using `id` and `name`.
* Update `JasmineTestFramework` to now use the new `TestSelection` format (still based on ID).
* Update `MochaTestFramework` to use the title to select tests.

Fixes #249

BREAKING CHANGES:

* Change api of `TestFramework`. It now provides an array of `TestSelection` objects, instead of an array of numbers with test ids.
Move code coverage intrumentation to a transpiler so we can handle the code transformation as all other transpilers. Simplifies the process by eliminating the need for keeping a seperate transformation process. Aso gets rid of the streaming api, so no need to know that part about node internals anymore.
@riezebosch
Copy link

Probably also fixes #291

Copy link
Member

@simondel simondel left a comment

Choose a reason for hiding this comment

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

It looks good! I have some minor comments about the code which you could resolve.

expect(result[2].fullName).eq('outer test 3');
});

function filter(testIds: number[]) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you ever pass anything other than a single number? If you don't won't it be easier to just allow one test id?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I've added a test where we filter 2 tests just to make sure it works.

realAddTest.apply(this, arguments);
}
current++;
Copy link
Member

Choose a reason for hiding this comment

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

You've removed this line but the declaration is still there. We can remove it right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I'll remove it.

expect(result.passes[0].fullTitle).eq('outer inner test 3');
});

function filter(testIds: number[]) {
Copy link
Member

Choose a reason for hiding this comment

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

The same filter comment for the Jasmine test applies here

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I've added a test where we filter 2 tests just to make sure it works.

@simondel simondel merged commit bb7c02f into master Oct 20, 2017
@simondel simondel deleted the fix-mocha-test-selection branch October 20, 2017 11:50
nicojs added a commit that referenced this pull request Jan 9, 2018
Support running mocha tests in Karma. It basically already worked since
#413, just remove the old
measures we set in place.
simondel pushed a commit that referenced this pull request Jan 12, 2018
Support running mocha tests in Karma. It basically already worked since
#413, just remove the old
measures we set in place.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants