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.

Sign up for GitHub

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

Jump to bottom

Fix failing tests with cross-env #639

Merged
merged 4 commits into from
Jul 18, 2023
Merged

Fix failing tests with cross-env #639

merged 4 commits into from
Jul 18, 2023

Conversation

bcomnes
Copy link
Contributor

@bcomnes bcomnes commented Jul 6, 2023

Trying to fix CI

@bcomnes bcomnes force-pushed the fix-ci branch 2 times, most recently from b1e7462 to 81c0c13 Compare July 6, 2023 16:54
@bcomnes bcomnes mentioned this pull request Jul 6, 2023
Open
4 tasks
@bcomnes bcomnes mentioned this pull request Jul 6, 2023
Merged
4 tasks
bcomnes
bcomnes commented Jul 14, 2023
View reviewed changes
package.json Outdated
"unit:templates-without-ts-esm": "tap \"templates/app/**/*.test.js\" \"templates/app-esm/**/*.test.js\" \"templates/app-ts/**/*.test.ts\" --no-coverage --timeout 200",
"unit:template-ts-esm": " cross-env TS_NODE_PROJECT=./templates/app-ts-esm/tsconfig.json tap templates/app-ts-esm/test/**/*.test.ts --no-coverage --node-arg=--loader=ts-node/esm --timeout 100 --color -R specy",
"unit:cli": "tap \"test/**/*.test.{js,ts}\" --no-coverage --jobs=1 --timeout 400 --color -R specy",
"unit:templates-without-ts-esm": "tap \"templates/app/**/*.test.js\" \"templates/app-esm/**/*.test.js\" \"templates/app-ts/**/*.test.ts\" --no-coverage --timeout 200 --color -R specy",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make the test output in CI much more readable and useful.

bcomnes
bcomnes commented Jul 14, 2023
View reviewed changes
package.json Outdated
"unit:template-ts-esm": "TS_NODE_PROJECT=./templates/app-ts-esm/tsconfig.json tap templates/app-ts-esm/test/**/*.test.ts --no-coverage --node-arg=--loader=ts-node/esm --timeout 100",
"unit:cli": "tap \"test/**/*.test.{js,ts}\" --no-coverage --timeout 200",
"unit:templates-without-ts-esm": "tap \"templates/app/**/*.test.js\" \"templates/app-esm/**/*.test.js\" \"templates/app-ts/**/*.test.ts\" --no-coverage --timeout 200",
"unit:template-ts-esm": " cross-env TS_NODE_PROJECT=./templates/app-ts-esm/tsconfig.json tap templates/app-ts-esm/test/**/*.test.ts --no-coverage --node-arg=--loader=ts-node/esm --timeout 100 --color -R specy",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests were failing when an ENV var went into the test cycle, which doesn't work on windows.

bcomnes
bcomnes commented Jul 14, 2023
View reviewed changes
test/start.test.js Outdated Show resolved Hide resolved
bcomnes
bcomnes commented Jul 14, 2023
View reviewed changes
test/start.test.js
@@ -556,7 +556,7 @@ test('should start the server listening on 0.0.0.0 when running in kubernetes',
t.pass('server closed')
})

test('should start the server with watch options that the child process restart when directory changed', { skip: process.platform === 'win32' }, async (t) => {
test('should start the server with watch options that the child process restart when directory changed', { skip: process.platform === 'win32' || (process.platform === 'darwin' && ['v20', 'v19', 'v18'].some(v => process.version.startsWith(v))) }, async (t) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We skip this on windows. Can we skip on macos?

Copy link
Member

Choose a reason for hiding this comment

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

go ahead

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer not to tbh, but I don't have good arguments for it.

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'll spend some time to try and debug. I don't understand what causes failure here tbh

mcollina
mcollina approved these changes Jul 15, 2023
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@bcomnes
Copy link
Contributor Author

bcomnes commented Jul 17, 2023

Ill revisit the skipped tests. My impression is that they are tightly coupled to platform/runtime assumptions and maybe its fair to skip where those assumptions differ.

mcollina
mcollina approved these changes Jul 18, 2023
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit eaa2dc3 into fastify:master Jul 18, 2023
@bcomnes bcomnes mentioned this pull request Jul 20, 2023
Closed
4 tasks
@bcomnes bcomnes deleted the fix-ci branch July 20, 2023 17:22
renovate bot referenced this pull request in specfy/specfy Aug 13, 2023
@renovate
chore(deps): update dependency fastify-cli to v5.8.0 (#142)
2977d8c 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [fastify-cli](https://github.com/fastify/fastify-cli) | [`5.7.1` ->
`5.8.0`](https://renovatebot.com/diffs/npm/fastify-cli/5.7.1/5.8.0) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/fastify-cli/5.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/fastify-cli/5.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/fastify-cli/5.7.1/5.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/fastify-cli/5.7.1/5.8.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>fastify/fastify-cli (fastify-cli)</summary>

###
[`v5.8.0`](https://github.com/fastify/fastify-cli/releases/tag/v5.8.0)

[Compare
Source](https://github.com/fastify/fastify-cli/compare/v5.7.1...v5.8.0)

##### What's Changed

- fix: Correct wrong instructions for typescript ejecting by
[@&#8203;anyonecancode](https://github.com/anyonecancode) in
[https://github.com/fastify/fastify-cli/pull/589](https://github.com/fastify/fastify-cli/pull/589)
- Improve the Typescript template. by
[@&#8203;meotimdihia](https://github.com/meotimdihia) in
[https://github.com/fastify/fastify-cli/pull/592](https://github.com/fastify/fastify-cli/pull/592)
- chore(.gitignore): add bun lockfile by
[@&#8203;Fdawgs](https://github.com/Fdawgs) in
[https://github.com/fastify/fastify-cli/pull/594](https://github.com/fastify/fastify-cli/pull/594)
- feat: add support for typescript and esm generation
([#&#8203;577](https://github.com/fastify/fastify-cli/issues/577)) by
[@&#8203;S-Abhishek](https://github.com/S-Abhishek) in
[https://github.com/fastify/fastify-cli/pull/585](https://github.com/fastify/fastify-cli/pull/585)
- fix: use dot notation by [@&#8203;is2ei](https://github.com/is2ei)
in
[https://github.com/fastify/fastify-cli/pull/634](https://github.com/fastify/fastify-cli/pull/634)
- Fix failing tests with cross-env by
[@&#8203;bcomnes](https://github.com/bcomnes) in
[https://github.com/fastify/fastify-cli/pull/639](https://github.com/fastify/fastify-cli/pull/639)
- print-routes options and print-plugins by
[@&#8203;bcomnes](https://github.com/bcomnes) in
[https://github.com/fastify/fastify-cli/pull/637](https://github.com/fastify/fastify-cli/pull/637)
- chore(deps): bump pino-pretty from 9.4.1 to 10.1.0 by
[@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/fastify/fastify-cli/pull/645](https://github.com/fastify/fastify-cli/pull/645)
- chore(deps-dev): bump
[@&#8203;types/node](https://github.com/types/node) from 18.17.0 to
20.4.4 by [@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/fastify/fastify-cli/pull/646](https://github.com/fastify/fastify-cli/pull/646)
- fix error on undefined plugin by
[@&#8203;dancastillo](https://github.com/dancastillo) in
[https://github.com/fastify/fastify-cli/pull/649](https://github.com/fastify/fastify-cli/pull/649)

##### New Contributors

- [@&#8203;anyonecancode](https://github.com/anyonecancode) made their
first contribution in
[https://github.com/fastify/fastify-cli/pull/589](https://github.com/fastify/fastify-cli/pull/589)
- [@&#8203;S-Abhishek](https://github.com/S-Abhishek) made their first
contribution in
[https://github.com/fastify/fastify-cli/pull/585](https://github.com/fastify/fastify-cli/pull/585)
- [@&#8203;is2ei](https://github.com/is2ei) made their first
contribution in
[https://github.com/fastify/fastify-cli/pull/634](https://github.com/fastify/fastify-cli/pull/634)
- [@&#8203;dancastillo](https://github.com/dancastillo) made their
first contribution in
[https://github.com/fastify/fastify-cli/pull/649](https://github.com/fastify/fastify-cli/pull/649)

**Full Changelog**:
fastify/fastify-cli@v5.7.1...v5.8.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 4pm on friday,before 9am on
monday,every weekend" in timezone Europe/Paris, Automerge - At any time
(no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, 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 has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/specfy/specfy).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi40MC4zIiwidXBkYXRlZEluVmVyIjoiMzYuNDAuMyIsInRhcmdldEJyYW5jaCI6ImNob3JlL3Jlbm92YXRlQmFzZUJyYW5jaCJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@Summonshr Summonshr mentioned this pull request Jun 29, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bcomnes @mcollina