Skip to content

Introducing module-path-filter and test-file-path-filter in ember-exam #266

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

Merged
merged 1 commit into from
Jun 4, 2019

Conversation

choheekim
Copy link
Collaborator

@choheekim choheekim commented Apr 22, 2019

ember-test provides filter and modules to allow executing a subset of the whole test suite. The problem of current provided options is that the filtering is processed after loading a test module, which means that the filtering is done in QUnit level after requiring a module. This results requiring all the test modules in the test suite even though a subset of the tests is targeted to execute.

module-path-filter and test-file-path-filter option provide to allow running a targeted test modules that are matched with a given module path string and it only loads/requires the targeted test modules which could impact reducing test loading time.

@choheekim
Copy link
Collaborator Author

choheekim commented Apr 22, 2019

Working on validating and adding tests against the option and will be adding design docs soon.

@choheekim choheekim force-pushed the module-filter branch 5 times, most recently from 0b7aed5 to 3aab1e6 Compare April 30, 2019 18:35
Copy link
Contributor

@stefanpenner stefanpenner left a comment

Choose a reason for hiding this comment

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

Left some small nit-picks, but others (assuming this works as expected) this LGTM!

Copy link
Collaborator

@step2yeung step2yeung left a comment

Choose a reason for hiding this comment

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

Would love to see docs for this as well! Otherwise, some comments but looking forward to this!!

@choheekim choheekim changed the title [WIP] Introducing module-path-filter in ember-exam Introducing module-path-filter in ember-exam May 8, 2019
@stefanpenner
Copy link
Contributor

LGTM after #266 (comment) is addressed.

@choheekim choheekim requested a review from step2yeung May 9, 2019 21:45
@choheekim choheekim changed the title Introducing module-path-filter in ember-exam [WIP] Introducing module-path-filter in ember-exam May 16, 2019
@choheekim choheekim changed the title [WIP] Introducing module-path-filter in ember-exam [WIP] Introducing module-path-filter and test-file-path-filter in ember-exam May 16, 2019
@choheekim choheekim force-pushed the module-filter branch 2 times, most recently from 42f3429 to c021f1c Compare May 21, 2019 01:11
@choheekim
Copy link
Collaborator Author

Update:

  • Added test-file-path-filter to provide ability of filtering test modules by given test file path
  • The regex to convert test file path to module path contains positive lookbehind and it seems not to be supported in javascript. (Trying to find alternative one)

@choheekim choheekim force-pushed the module-filter branch 3 times, most recently from 6539937 to a65665e Compare May 23, 2019 17:07
@choheekim
Copy link
Collaborator Author

choheekim commented May 23, 2019

New features introduced so far in this pr:
Two options

  1. module-path-filter
  2. test-file-path-filter

The two options filter test modules by either module path or test file path. Users can either provide module path in a form of string or regex or test file path. For an option test-file-path it also supports wildcard, e.g. --test-file-path-filter=/tests/unit/* will execute all test files under /tests/unit/.

Soon to update to support to allow multiple paths, e.g. --module-path-filter=weight, filter will execute test modules that have either weight and filter in the module path.

@choheekim choheekim force-pushed the module-filter branch 2 times, most recently from 51cc373 to f7e2aed Compare May 29, 2019 19:40
Copy link
Contributor

@stefanpenner stefanpenner left a comment

Choose a reason for hiding this comment

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

this LGTM, any further considerations or are we ready to move forward?

@choheekim
Copy link
Collaborator Author

this LGTM, any further considerations or are we ready to move forward?

It's ready to move forward but I'd love to get another approval from @step2yeung. What do you think?

Copy link
Collaborator

@step2yeung step2yeung left a comment

Choose a reason for hiding this comment

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

This is looking good, can remove the WIP tag? Could we get docs for this as well?

@choheekim
Copy link
Collaborator Author

choheekim commented Jun 3, 2019

Status: Working on addressing review comments and updating docs

@choheekim
Copy link
Collaborator Author

This is looking good, can remove the WIP tag? Could we get docs for this as well?

Yes, I'm currently working on docs :)

@choheekim choheekim changed the title [WIP] Introducing module-path-filter and test-file-path-filter in ember-exam Introducing module-path-filter and test-file-path-filter in ember-exam Jun 3, 2019
@choheekim choheekim force-pushed the module-filter branch 3 times, most recently from c82d317 to 8184515 Compare June 3, 2019 21:55
@choheekim
Copy link
Collaborator Author

@stefanpenner @step2yeung Docs has been updated and all comments are addressed :)

@stefanpenner
Copy link
Contributor

@choheekim nice, looks like it needs a rebase. Mind doing that, at which point I'm good with us shipping this.

@choheekim
Copy link
Collaborator Author

@choheekim nice, looks like it needs a rebase. Mind doing that, at which point I'm good with us shipping this.

Rebased and CI is passing now :) Ready to be shipped

@step2yeung step2yeung merged commit dbc32a0 into ember-cli:master Jun 4, 2019
@choheekim choheekim deleted the module-filter branch September 30, 2019 20:13
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.

None yet

4 participants