Skip to content

Conversation

@martinm82
Copy link
Contributor

No description provided.

@martinm82 martinm82 changed the title ci: run tests on CI ci: run tests on pull requests Jan 5, 2023
"test": "npm-run-all --print-label --parallel lint:* --parallel test:*",
"lint:es": "eslint .",
"test": "npm-run-all --print-label --parallel test:*",
"citest": "npm-run-all --print-label --parallel \"test:* -- --reporters=github-actions\"",
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this also run test:me, test:unit:watch, test:integration:debug ? Maybe not a problem since npm test worked fine locally using the same argument to npm-run-all..

"start": "probot run ./index.js",
"test": "npm-run-all --print-label --parallel lint:* --parallel test:*",
"lint:es": "eslint .",
"test": "npm-run-all --print-label --parallel test:*",
Copy link
Contributor

Choose a reason for hiding this comment

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

Great work! With lint gone from npm test it would be awesome to add either a CONTRIBUTING.md file (or a simple "Development" heading in the repo readme) that states that it's expected to successfully run npm lint and npm test before posting a PR.

@decyjphr
Copy link
Collaborator

decyjphr commented Jan 5, 2023

@martinm82 I just started looking at this and enabled to workflow to run.
You'll see that the workflow ran with errors.
If I get the tests to pass using the merge-pr-375 branch , we can get the merge done.

@robandpdx
Copy link
Contributor

@martinm82 I just started looking at this and enabled to workflow to run. You'll see that the workflow ran with errors. If I get the tests to pass using the merge-pr-375 branch , we can get the merge done.

@decyjphr I would love to see the unit test passing.

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.

4 participants