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

Update live/watched test documentation and scripts #5467

Merged
merged 3 commits into from
May 17, 2023
Merged

Conversation

lyzadanger
Copy link
Contributor

This PR:

For reference, these commands will all run and watch tests as of these changes (in order of what I think of as most conventional and convenient to least):

  • yarn test:watch
  • gulp test --live
  • npm run test:watch
  • make test ARGS='--live'

Testing these changes

  • Check out branch, update dependencies
  • Run the four commands listed above. All should work (run the full suite of tests and then watch for source file changes)
  • Run make docs. This should build the updated RST docs with sphinx and open a webpage to http://127.0.0.1:8000/ with the documentation content.

This impacts the `--watch` flag to the `gulp test` task
This can help abstract the `--watch` - `--live` change in updated
`frontend-build` package `tests` task.
Comment on lines -112 to -114
You can filter the tests which are run by running ``make test ARGS='--grep <pattern>'``,
or ``yarn test --grep <pattern>``. Only test files matching the regex ``<pattern>`` will
be executed.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the make variant here not because it's not true (we can add it back if so desired) but just because I'm trying to minimize complexity. The convention I'm going for here is to lean toward make for full runs of things (e.g. make test, make sure) and yarn for filtered runs. We can discuss this and tweak as desired.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's in fact how I usually do it. It seems like a sensible and intuitive approach.

@lyzadanger
Copy link
Contributor Author

Note: Other sections on the developing.rst page need updating, too. I was honed in only on test-running.

@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #5467 (4794174) into main (0bda420) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #5467   +/-   ##
=======================================
  Coverage   99.21%   99.21%           
=======================================
  Files         238      238           
  Lines        9388     9388           
  Branches     2237     2237           
=======================================
  Hits         9314     9314           
  Misses         74       74           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@lyzadanger lyzadanger requested a review from acelaya May 16, 2023 12:56
Copy link
Contributor

@acelaya acelaya left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines -112 to -114
You can filter the tests which are run by running ``make test ARGS='--grep <pattern>'``,
or ``yarn test --grep <pattern>``. Only test files matching the regex ``<pattern>`` will
be executed.
Copy link
Contributor

Choose a reason for hiding this comment

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

That's in fact how I usually do it. It seems like a sensible and intuitive approach.

Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

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

We plan to replace gulp in future, which will solve the problem with conflicting flag names more generally, but I'm approving this as a pragmatic solution in the interim.

@lyzadanger lyzadanger merged commit e9ad395 into main May 17, 2023
4 checks passed
@lyzadanger lyzadanger deleted the update-test-docs branch May 17, 2023 12:05
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

3 participants