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

chore(www): add a --watch flag for local test debugging #1712

Merged
merged 4 commits into from
Sep 20, 2023

Conversation

sbruens
Copy link
Contributor

@sbruens sbruens commented Sep 18, 2023

Right now when running npm run action www/test you get a single run by default, which is useful for CI workflows. I'm adding a --watch flag to help in debugging by keeping the server alive and re-running tests on changes.

@sbruens sbruens requested a review from a team as a code owner September 18, 2023 16:39
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Patch has no changes to coverable lines.

📢 Thoughts on this report? Let us know!.

@sbruens sbruens changed the base branch from master to sbruens/coverage September 20, 2023 03:22
Copy link
Contributor

@daniellacosse daniellacosse left a comment

Choose a reason for hiding this comment

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

I would much prefer we do one of the following to keep behavior consistent across actions:

  1. do the inverse behind a --watch flag
  2. modify all the actions to default to a watch process and take a --ci flag for the single run case

* @param {string[]} parameters The list of action arguments passed in.
* @returns {Object} Object containing whether to run as part of CI.
*/
function getActionParameters(cliArguments) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I kinda feel like this function just adds indirection without much benefit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point. I mostly did this to keep it consistent with some of the other scripts I saw using this type of helper function. Removing it.

Copy link
Contributor

@daniellacosse daniellacosse Sep 20, 2023

Choose a reason for hiding this comment

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

In those cases the helper functions are shared across actions

@sbruens sbruens changed the base branch from sbruens/coverage to master September 20, 2023 20:04
@sbruens
Copy link
Contributor Author

sbruens commented Sep 20, 2023

I would much prefer we do one of the following to keep behavior consistent across actions:

  1. do the inverse behind a --watch flag
  2. modify all the actions to default to a watch process and take a --ci flag for the single run case

Done. Went with the --watch flag.

@sbruens sbruens changed the title chore(www): add a --ci flag for single runs chore(www): add a --watch flag for local test debugging Sep 20, 2023
@sbruens sbruens merged commit 8307952 into master Sep 20, 2023
16 checks passed
@sbruens sbruens deleted the sbruens/test-dev branch September 20, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants