allow list of search patterns in testFiles config option #5402

bahmutov · 2019-10-21T13:53:11Z

Closes Allow passing a list of glob patterns in testFiles config option #5401

User facing changelog

allows a list of specs when searching for test files via config argument testFiles which follows the principle in ignoreTestFiles config option

Additional details

Using glob to select several groups of spec names is not obvious. For example to select both .md and *spec.js files one would need to write without spaces

{
  "testFiles": "{*.md,*spec.js}"
}

With this change, user could write

{
  "testFiles": ["*.md", "*spec.js"]
}

which to me looks more natural and easier to remember. Also added debug log messages during DEBUG=cypress:server:specs that show how the search happens in case on needs to debug it.

How has the user experience changed?

PR Tasks

Have tests been added/updated?
Has a PR for user-facing changes been opened in cypress-documentation? Issue 2183 cypress-documentation#2184
Have API changes been updated in the type definitions?
Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2019-10-21T13:53:13Z

Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.

Please write [WIP] in the title of your Pull Request if your PR is not ready for review - someone will review your PR as soon as the [WIP] is removed.
Please familiarize yourself with the PR Review Checklist and feel free to make updates on your PR based on these guidelines.

PR Review Checklist

If any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'.

User Experience

The feature/bugfix is self-documenting from within the product.
The change provides the end user with a way to fix their problem (no dead ends).

Functionality

The code works and performs its intended function with the correct logic.
Performance has been factored in (for example, the code cleans up after itself to not cause memory leaks).
The code guards against edge cases and invalid input and has tests to cover it.

Maintainability

The code is readable (too many nested 'if's are a bad sign).
Names used for variables, methods, etc, clearly describe their function.
The code is easy to understood and there are relevant comments explaining.
New algorithms are documented in the code with link(s) to external docs (flowcharts, w3c, chrome, firefox).
There are comments containing link(s) to the addressed issue (in tests and code).

Quality

The change does not reimplement code.
There's not a module from the ecosystem that should be used instead.
There is no redundant or duplicate code.
There are no irrelevant comments left in the code.
Tests are testing the code’s intended functionality in the best way possible.

Internal

The original issue has been tagged with a release in ZenHub.

bahmutov · 2019-10-21T15:32:42Z

packages/server/test/unit/specs_spec.coffee

          path.join(cfg.projectRoot, "cypress", "integration", "js_spec.js")
-        ])
+        ]
+        specsUtil.find(cfg, specPattern)


being a little bit clearer on the arguments

bahmutov · 2019-10-21T15:33:08Z

packages/server/test/unit/specs_spec.coffee

      .then (files) ->
        expect(files.length).to.equal(1)
        expect(files[0].name).to.equal("js_spec.js")

    it "filters using specPattern as array of glob patterns", ->
      config.get(FixturesHelper.projectPath("various-file-types"))
      .then (cfg) ->
-        specsUtil.find(cfg, [
+        debug("test config testFiles is %o", cfg.testFiles)


adding a few debug statements to the test itself is useful when iterating on the test and understanding what is going on

bahmutov · 2019-10-21T15:33:43Z

packages/server/lib/util/specs.js

-  .tap((files) => {
-    return debug('found %d spec files: %o', files.length, files)
-  })
+  return Promise.mapSeries(testFilesPatterns, findOnePattern).then(_.flatten)


find matching spec files one pattern at a time and then merging them

cli/schema/cypress.schema.json

Change `testFiles` from a string to either a string **or** array of strings; following similar pattern as `ignoreTestFiles`

See paulmillr/chokidar#855 for more details.

Seems to fail for similar reason as #4543 and is likely resolved with PR #4849

CLAassistant · 2019-10-21T20:29:53Z

All committers have signed the CLA.

andrew-codes · 2019-10-21T20:35:08Z

packages/server/test/integration/cypress_spec.coffee

+    it "runs project by limiting spec files via config.testFiles as an array of string glob patterns", ->
+      cypress.start(["--run-project=#{@todosPath}", "--config=testFiles=**/test2.coffee,**/test1.js"])
+      .then =>
+        expect(browsers.open).to.be.calledWithMatch(ELECTRON_BROWSER, {url: "http://localhost:8888/__/#/tests/integration/test2.coffee"})
+      .then =>
+        expect(browsers.open).to.be.calledWithMatch(ELECTRON_BROWSER, {url: "http://localhost:8888/__/#/tests/integration/test1.js"})
+        @expectExitWith(0)
+
+    it "runs project by limiting spec files via config.testFiles as an array of string glob patterns", ->
+      cypress.start(["--run-project=#{@todosPath}", "--config='[\"testFiles=**/test2.coffee\",\"**/test1.js\"]'"])
+      .then =>
+        expect(browsers.open).to.be.calledWithMatch(ELECTRON_BROWSER, {url: "http://localhost:8888/__/#/tests/integration/test2.coffee"})
+      .then =>
+        expect(browsers.open).to.be.calledWithMatch(ELECTRON_BROWSER, {url: "http://localhost:8888/__/#/tests/integration/test1.js"})
+        @expectExitWith(0)
+


@bahmutov per the commit message, I'll echo it here, I think these fail for similar reasons as #4896. There is an outstanding PR for this issue. I can review the PR, get it merged when ready, and then update this branch to see if it resolves this issue.

I retract my assessment. It looks like we do not need the functionality in that PR. Furthermore, supporting the syntax --config=testFiles=["**/*.js", "**/*.coffee"] is how other mutli-value config values work; such as ignoreTestFiles. I do not see any documentation or elsewhere in tests indicating that the format, --config=testFiles=**/*.js,**/*.coffee, is actually supported.

Distinguish between the two tests; as they are subtly different.

bahmutov · 2019-10-21T20:48:49Z

cli/types/cypress-npm-api.d.ts

@@ -43,7 +43,7 @@ declare module 'cypress' {
     */
    reporter: string,
    /**
-     * A String glob pattern of the test files to load.
+     * A String or Array of string glob pattern of the test files to load.


got to update the type here as well

testFiles: string | string[]

Updated the comments, but forgot to update the actual type. This commit rectifies that.

`--config` was not being passed the appropriate value. Now matches other test cases; such as that found in [args_spec](https://github.com/cypress-io/cypress/blob/92b91fe514e5ff6286b4d4e26d2df23062bdf869/packages/server/test/unit/args_spec.coffee#L210)

From looking at other tests, it does not appear that the syntax `--config=testFiles=glob1,glob2` is current supported. Removing the test for this test case.

packages/server/test/integration/cypress_spec.coffee

bahmutov · 2019-10-22T14:38:42Z

packages/server/test/unit/config_spec.coffee

@@ -322,9 +322,18 @@ describe "lib/config", ->
          @setup({testFiles: "**/*.coffee"})
          @expectValidationPasses()

-        it "fails if not a string", ->
+        it "passes if an array of strings", ->
+          @setup({ignoreTestFiles: ["**/*.coffee", "**/*.jsx"]})


you mean testFiles here right ;)

Yup, let me change these two places within config_spec

bahmutov · 2019-10-22T14:38:59Z

packages/server/test/unit/config_spec.coffee

+          @expectValidationFails("be a string or an array of string")
+
+        it "fails if not an array of strings", ->
+          @setup({ignoreTestFiles: [5]})


you mean testFiles

bahmutov

see two comments in tests

Previously was testing `ignoreTestFiles` instead of `testFies`

cli/schema/cypress.schema.json

packages/server/test/integration/cypress_spec.coffee

Content updates due ot changes in [PR](cypress-io/cypress#5402)

Should be a string or an array of strings.

packages/server/test/integration/cypress_spec.coffee

cli/schema/cypress.schema.json

kuceb

accept that wording change if you want. but LGTM

kuceb · 2019-10-29T14:42:09Z

make sure to merge in develop and rebuild after we get this circle zip file size figured out

Co-Authored-By: Ben Kucera <14625260+Bkucera@users.noreply.github.com>

* update docs to reflect changes to `testFiles` Content updates due ot changes in [PR](cypress-io/cypress#5402) * examples of multi-value config properties via CLI Co-authored-by: Jennifer Shehane <jennifer@cypress.io>

WIP: log spec search steps

2b9f1c3

bahmutov changed the title ~~WIP: log spec search steps~~ WIP: allow list of search patterns in testFiles config option Oct 21, 2019

bahmutov added 2 commits October 21, 2019 11:24

add multiple files via glob test

9f9d359

allow testFiles to be a string or a list of strings to match

34c90b4

bahmutov commented Oct 21, 2019

View reviewed changes

cli/schema/cypress.schema.json Outdated Show resolved Hide resolved

bahmutov and others added 6 commits October 21, 2019 11:47

Merge branch 'develop' into list-spec-files-5401

9272570

failing config tests: testFiles type change

afd0e84

Change `testFiles` from a string to either a string **or** array of strings; following similar pattern as `ignoreTestFiles`

testFiles type change passing tests

07aa56b

update verbiage related to testFiles type change

137135c

enable running integration tests locally

e91b3c8

See paulmillr/chokidar#855 for more details.

integration tests; failing

a3db461

Seems to fail for similar reason as #4543 and is likely resolved with PR #4849

andrew-codes reviewed Oct 21, 2019

View reviewed changes

correct test verbiage

7837456

Distinguish between the two tests; as they are subtly different.

bahmutov commented Oct 21, 2019

View reviewed changes

andrew-codes added 3 commits October 21, 2019 17:19

update TS types for testFiles type change

3726073

Updated the comments, but forgot to update the actual type. This commit rectifies that.

correct CLI syntax in test

6428ded

`--config` was not being passed the appropriate value. Now matches other test cases; such as that found in [args_spec](https://github.com/cypress-io/cypress/blob/92b91fe514e5ff6286b4d4e26d2df23062bdf869/packages/server/test/unit/args_spec.coffee#L210)

remove unsupported syntax from tests

7a413c4

From looking at other tests, it does not appear that the syntax `--config=testFiles=glob1,glob2` is current supported. Removing the test for this test case.

andrew-codes self-assigned this Oct 21, 2019

andrew-codes changed the title ~~WIP: allow list of search patterns in testFiles config option~~ allow list of search patterns in testFiles config option Oct 22, 2019

bahmutov commented Oct 22, 2019

View reviewed changes

packages/server/test/integration/cypress_spec.coffee Show resolved Hide resolved

bahmutov commented Oct 22, 2019

View reviewed changes

bahmutov requested a review from kuceb October 22, 2019 14:39

test the correct config value

02e15a2

Previously was testing `ignoreTestFiles` instead of `testFies`

provide more accurate test assertion

e8926d6

kuceb suggested changes Oct 22, 2019

View reviewed changes

cli/schema/cypress.schema.json Outdated Show resolved Hide resolved

cli/schema/cypress.schema.json Outdated Show resolved Hide resolved

packages/server/test/integration/cypress_spec.coffee Outdated Show resolved Hide resolved

andrew-codes mentioned this pull request Oct 22, 2019

Update documentation based on changes to testFiles and provide examples of syntax for multi-value config options via CLI cypress-io/cypress-documentation#2183

Closed

andrew-codes added a commit to cypress-io/cypress-documentation that referenced this pull request Oct 22, 2019

update docs to reflect changes to testFiles

155ed83

Content updates due ot changes in [PR](cypress-io/cypress#5402)

andrew-codes mentioned this pull request Oct 22, 2019

Issue 2183 cypress-io/cypress-documentation#2184

Merged

andrew-codes added 2 commits October 22, 2019 12:01

correct config's testFiles type

38e1383

Should be a string or an array of strings.

remove unnecessary newline

ee7dd76

andrew-codes requested a review from kuceb October 22, 2019 16:03

kuceb suggested changes Oct 22, 2019

View reviewed changes

packages/server/test/integration/cypress_spec.coffee Outdated Show resolved Hide resolved

andrew-codes requested a review from kuceb October 23, 2019 15:02

Merge branch 'develop' into list-spec-files-5401

8e466c6

kuceb reviewed Oct 29, 2019

View reviewed changes

cli/schema/cypress.schema.json Outdated Show resolved Hide resolved

kuceb previously approved these changes Oct 29, 2019

View reviewed changes

tweak verbiage to better align with intention

ab9704e

Co-Authored-By: Ben Kucera <14625260+Bkucera@users.noreply.github.com>

andrew-codes dismissed kuceb’s stale review via ab9704e October 30, 2019 14:50

Merge branch 'develop' into list-spec-files-5401

bf16eca

kuceb approved these changes Oct 31, 2019

View reviewed changes

jennifer-shehane merged commit b3e40b0 into develop Oct 31, 2019

cypress-bot bot mentioned this pull request Oct 31, 2019

Allow passing a list of glob patterns in testFiles config option #5401

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow list of search patterns in testFiles config option #5402

allow list of search patterns in testFiles config option #5402

bahmutov commented Oct 21, 2019 •

edited by jennifer-shehane

cypress-bot bot commented Oct 21, 2019

bahmutov Oct 21, 2019

bahmutov Oct 21, 2019

bahmutov Oct 21, 2019

CLAassistant commented Oct 21, 2019 •

edited

andrew-codes Oct 21, 2019

bahmutov Oct 21, 2019

andrew-codes Oct 22, 2019

bahmutov Oct 21, 2019

bahmutov Oct 22, 2019

andrew-codes Oct 22, 2019

bahmutov Oct 22, 2019

bahmutov left a comment

kuceb left a comment

kuceb commented Oct 29, 2019

allow list of search patterns in testFiles config option #5402

allow list of search patterns in testFiles config option #5402

Conversation

bahmutov commented Oct 21, 2019 • edited by jennifer-shehane

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Oct 21, 2019

PR Review Checklist

User Experience

Functionality

Maintainability

Quality

Internal

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CLAassistant commented Oct 21, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bahmutov left a comment

Choose a reason for hiding this comment

kuceb left a comment

Choose a reason for hiding this comment

kuceb commented Oct 29, 2019

bahmutov commented Oct 21, 2019 •

edited by jennifer-shehane

CLAassistant commented Oct 21, 2019 •

edited