Skip to content

chore: Split test and lint scripts#297

Merged
karfau merged 3 commits into
masterfrom
split-test-and-lint
Aug 24, 2021
Merged

chore: Split test and lint scripts#297
karfau merged 3 commits into
masterfrom
split-test-and-lint

Conversation

@karfau

@karfau karfau commented Aug 23, 2021

Copy link
Copy Markdown
Member

To fix #111 I think it's good enough to split test and lint into two different build steps.
Separate actions are not required since the github actions details view now shows what steps failed.
image

To be able to do that split, I needed to drop the execution of the lint script from the test script. The start script still runs both in watch mode, so it's still easy to run both when developing.

@karfau karfau requested a review from brody2consult August 23, 2021 20:35
To fix #111 I think it's good enough to split test and lint into two different build steps.
Separate actions are not required since the github actions details view now shows what steps failed.

To be able to do that split, I needed to drop the execution of the `lint` script from the `test` script. The `start` script still runs both in watch mode, so it's still easy to run both when developing.

@brody2consult brody2consult left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks to me like npm-run-all is no longer needed and should be removed ?

@karfau

karfau commented Aug 24, 2021

Copy link
Copy Markdown
Member Author

@brodybits you are right, I dropped it, quite some reduction on devDependencies side

$ npm rm npm-run-all
removed 49 packages, updated 5 packages and audited 600 packages in 12.223s

@karfau karfau requested a review from brody2consult August 24, 2021 17:59

@brody2consult brody2consult left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM overall

with:
node-version: ${{ matrix.node-version }}
- run: npm cit
- run: npm ci --no-audit

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

minor question: Can you quickly describe the motivation behind adding --no-audit here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes:
a) (Since we have tools to provide update PRs) nobody is looking at the audit output on CI
b) it saves time to not do it

@karfau karfau merged commit 6cc16a6 into master Aug 24, 2021
@karfau karfau deleted the split-test-and-lint branch August 24, 2021 18:19
@brody2consult

Copy link
Copy Markdown
Member

@karfau it looks like this broke the Stryker run in master ... definitely not the first time we hit this kind of thing.

I am wondering if using Stryker Jest runner, as I tried in PR #208, could help avoid this failure.

I am thinking now if we could find a way to add another task to check that Stryker can do its initial run in the future PRs?

@karfau

karfau commented Aug 24, 2021

Copy link
Copy Markdown
Member Author

I am thinking now if we could find a way to add another task to check that Stryker can do its initial run in the future PRs?

Yes, also thought about this, but I couldn't find any documentation about such a "dryrun" mode yet.

Update: I filed stryker-mutator/stryker-js#3088

karfau added a commit that referenced this pull request Aug 24, 2021
by pointing it to the existing test script instead of the missing `test:jest` srcipt that has been removed in #297
@karfau karfau mentioned this pull request Aug 24, 2021
@karfau karfau added this to the next release milestone Aug 28, 2021
karfau pushed a commit that referenced this pull request May 9, 2026
This PR contains the following updates:

| Package | Change |
[Age](https://docs.renovatebot.com/merge-confidence/) |
[Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [auto-changelog](https://redirect.github.com/CookPete/auto-changelog)
| [`2.5.0` →
`2.5.1`](https://renovatebot.com/diffs/npm/auto-changelog/2.5.0/2.5.1) |
![age](https://developer.mend.io/api/mc/badges/age/npm/auto-changelog/2.5.1?slim=true)
|
![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/auto-changelog/2.5.0/2.5.1?slim=true)
|

---

### Release Notes

<details>
<summary>CookPete/auto-changelog (auto-changelog)</summary>

###
[`v2.5.1`](https://redirect.github.com/CookPete/auto-changelog/blob/HEAD/CHANGELOG.md#v251)

[Compare
Source](https://redirect.github.com/CookPete/auto-changelog/compare/v2.5.0...v2.5.1)

- \[Fix] corrected base and target references in devops compare url
[`#297`](https://redirect.github.com/cookpete/auto-changelog/issues/297)
- Only apps should have lockfiles
[`54bbb31`](https://redirect.github.com/cookpete/auto-changelog/commit/54bbb31efa81f115bc27f34f7cb4e41aec2385de)
- \[Tests] migrate from `mocha`+`chai` to `tape`
[`147be7e`](https://redirect.github.com/cookpete/auto-changelog/commit/147be7e31cd95875e40ef1cc4276c9131a20bade)
- \[Tests] migrate from travis to GHA
[`79cd61a`](https://redirect.github.com/cookpete/auto-changelog/commit/79cd61a1f619b1fc96880af163a59319b4bff33d)

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/xmldom/xmldom).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xNTkuMiIsInVwZGF0ZWRJblZlciI6IjQzLjE1OS4yIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

keep lint in separate GitHub action

2 participants