Skip to content
This repository was archived by the owner on Jan 14, 2024. It is now read-only.

describe tests in README; use jest projects#169

Merged
SallyMcGrath merged 3 commits intomasterfrom
readme_js1wk1
Sep 6, 2021
Merged

describe tests in README; use jest projects#169
SallyMcGrath merged 3 commits intomasterfrom
readme_js1wk1

Conversation

@gregdyke
Copy link
Copy Markdown
Contributor

@gregdyke gregdyke commented Jul 3, 2021

npm test now runs all tests. Running only the mandatory tests is described in the README.

Suggestion for when we have auto running of tests on commit would be to add an npm script that is run with npm run post-commit or something like that (that would only run the mandatory tests and not the extra?)


View rendered README.md

@gregdyke gregdyke requested a review from a team July 3, 2021 02:19
Copy link
Copy Markdown
Contributor

@40thieves 40thieves left a comment

Choose a reason for hiding this comment

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

Some suggestions, otherwise looking good 🙂

Comment thread README.md Outdated
Comment thread README.md
Comment on lines +23 to +27
Try out variant way of running tests:

- `npm test` -> run all mandatory and extra tests
- `npm test -- --selectProjects mandatory` -> run only mandatory tests
- `npm test -- --testPathPattern mandatory/1-syntax-errors.js` -> run single test
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This feels repetitive with the section above, what is it adding?

Comment thread README.md
- To run the tests for all mandatory/extra exercises, run `npm test`
- To run only the tests for the mandatory exercises, run `npm test -- --selectProjects mandatory`
- To run only the tests for the extra exercises, run `npm test -- --selectProjects extra`
- To run a single exercise/test (for example `mandatory/1-writer.js`), run `npm test -- --testPathPattern mandatory/1-writer.js` (Remember, you can use tab-completion to get files relative to the current directory, so m`Tab ↹`/1-`Tab ↹` will autocomplete get you the test file starting with 1-)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you think about using <kbd>? Perhaps a bit overly clever?

Suggested change
- To run a single exercise/test (for example `mandatory/1-writer.js`), run `npm test -- --testPathPattern mandatory/1-writer.js` (Remember, you can use tab-completion to get files relative to the current directory, so m`Tab ↹`/1-`Tab ↹` will autocomplete get you the test file starting with 1-)
- To run a single exercise/test (for example `mandatory/1-writer.js`), run `npm test -- --testPathPattern mandatory/1-writer.js` (Remember, you can use tab-completion to get files relative to the current directory, so <kbd>m</kbd><kbd>Tab ↹</kbd>/<kbd>1-</kbd><kbd>Tab ↹</kbd> will autocomplete get you the test file starting with 1-)

That would render as: mTab ↹/1-Tab ↹

Copy link
Copy Markdown
Member

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

LGTM - thanks!

@github-actions
Copy link
Copy Markdown

Your coursework submission has been closed because nobody has interacted with it in six weeks. You are welcome to re-open it to get more feedback.

@github-actions github-actions bot added the Stale label Aug 30, 2021
@github-actions github-actions bot closed this Aug 30, 2021
@illicitonion illicitonion reopened this Aug 30, 2021
@github-actions github-actions bot closed this Aug 31, 2021
@40thieves 40thieves reopened this Sep 6, 2021
@40thieves
Copy link
Copy Markdown
Contributor

We have stalebot problems!

@SallyMcGrath SallyMcGrath merged commit ed4603b into master Sep 6, 2021
@ChrisOwen101
Copy link
Copy Markdown
Contributor

@40thieves Seems like it's working exactly as intended to me 😉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants