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

feat(ci): run integration tests on windows #6279

Merged
merged 305 commits into from
Nov 20, 2023
Merged

feat(ci): run integration tests on windows #6279

merged 305 commits into from
Nov 20, 2023

Conversation

mehulkar
Copy link
Contributor

@mehulkar mehulkar commented Oct 25, 2023

This PR enables starts running our integration tests on Windows and fixes some of the
issues that come out of that. Mostly the issues are around finding binaries with .exe
extension (the test runner and turbo itself), file path separators, and line endings.

This PR adds a new Github Action "job" for both the Go and Rust variants, because
there are still a few remaining tests that are not passing that will need a little more
investigation. Once this diff is merged, we can individually look at each test (e.g. #6477)

Closes TURBO-1533

Some Prysk references

@vercel
Copy link

vercel bot commented Oct 25, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-basic-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 20, 2023 7:33pm
examples-gatsby-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Nov 20, 2023 7:33pm
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 20, 2023 7:33pm
examples-tailwind-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Nov 20, 2023 7:33pm
7 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-cra-web ⬜️ Ignored (Inspect) Visit Preview Nov 20, 2023 7:33pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Nov 20, 2023 7:33pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Nov 20, 2023 7:33pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Nov 20, 2023 7:33pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Nov 20, 2023 7:33pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Nov 20, 2023 7:33pm
turbo-site ⬜️ Ignored (Inspect) Visit Preview Nov 20, 2023 7:33pm

@github-actions
Copy link
Contributor

github-actions bot commented Oct 25, 2023

🟢 Turbopack Benchmark CI successful 🟢

Thanks

@mehulkar mehulkar changed the title windows only feat(ci): run integration tests on windows Oct 30, 2023
@@ -80,7 +80,7 @@ Validate that we got a full task summary for the failed task with an error in .e
"execution": {
"startTime": [0-9]+, (re)
"endTime": [0-9]+, (re)
"error": "command .* npm run maybefails exited \(1\)", (re)
"error": "command (.*)npm run maybefails exited \(1\)", (re)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This came from Rust tests, I thiink it's unrelated to this PR, but I was cross cehcking that test suite to make sure anything didn't break

Copy link
Member

@chris-olszewski chris-olszewski left a comment

Choose a reason for hiding this comment

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

I'd like to see the Windows integration tests get to running the integration test. Currently they're failing on building the binary.

I also don't love distributing our own compiled version of echo $@. I can accept that we might need an executable due to lack of Windows shebang support, but I think at minimum we should include the source in repo. In the ideal world we'd just add the compilation step as a task-dependency to the integration tests and avoid checking in binaries, but if that's not easily doable I think it's also okay to have a script that refreshes checked in binaries.

.github/workflows/test.yml Outdated Show resolved Hide resolved
- Add turbo.exe stub source into this repo
- Run it as a task dependency of integration tests
- Place the turbo.exe in the appropriate folders in the fixtures
@mehulkar
Copy link
Contributor Author

mehulkar commented Nov 20, 2023

I'd like to see the Windows integration tests get to running the integration test

It builds and runs the tests for the Go codepath, which is where I started a few weeks ago now. I've disabled the Rust codepath, which is failing to build. I'd like to debug that separately so we can get something merged (it might take me some time to look into this.

I also don't love distributing our own compiled version of echo $@. I can accept that we might need an executable due to lack of Windows shebang support, but I think at minimum we should include the source in repo. In the ideal world we'd just add the compilation step as a task-dependency to the integration tests and avoid checking in binaries, but if that's not easily doable I think it's also okay to have a script that refreshes checked in binaries.

Done, see #6508 or e010acb for details.

due to lack of Windows shebang support

For what it's worth I don't think this is the problem. Executing these echo $@ files manually from Bash works just fine, it's just through Rust that it seems to fail.

Copy link
Contributor

Linux Benchmark for 5267d8e

Test Base PR % Significant %
bench_startup/Turbopack CSR/1000 modules 1116.01ms ± 2.87ms 1080.03ms ± 6.42ms -3.22% -1.57%
Click to view full benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 21.58ms ± 0.86ms 21.52ms ± 0.84ms -0.30%
bench_hmr_to_eval/Turbopack CSR/1000 modules 21.23ms ± 0.84ms 21.15ms ± 0.84ms -0.39%
bench_startup/Turbopack CSR/1000 modules 1116.01ms ± 2.87ms 1080.03ms ± 6.42ms -3.22% -1.57%

Copy link
Contributor

Linux Benchmark for 9ea5945

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 22.99ms ± 0.95ms 23.01ms ± 0.96ms +0.09%
bench_hmr_to_eval/Turbopack CSR/1000 modules 22.49ms ± 0.96ms 22.77ms ± 1.02ms +1.23%
bench_startup/Turbopack CSR/1000 modules 1104.14ms ± 7.12ms 1091.56ms ± 8.64ms -1.14%

Copy link
Contributor

Linux Benchmark for bc8911f

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 22.69ms ± 0.93ms 22.67ms ± 0.96ms -0.05%
bench_hmr_to_eval/Turbopack CSR/1000 modules 22.44ms ± 0.80ms 22.29ms ± 1.02ms -0.68%
bench_startup/Turbopack CSR/1000 modules 1102.86ms ± 9.12ms 1087.61ms ± 14.96ms -1.38%

Copy link
Contributor

Linux Benchmark for b98b9ae

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 21.44ms ± 0.84ms 21.54ms ± 0.89ms +0.47%
bench_hmr_to_eval/Turbopack CSR/1000 modules 20.98ms ± 0.81ms 20.96ms ± 0.81ms -0.11%
bench_startup/Turbopack CSR/1000 modules 1102.80ms ± 6.07ms 1120.68ms ± 14.38ms +1.62%

Copy link
Contributor

Linux Benchmark for 2cc6b6e

Click to view benchmark
Test Base PR % Significant %
bench_hmr_to_commit/Turbopack CSR/1000 modules 23.17ms ± 0.97ms 23.24ms ± 1.01ms +0.32%
bench_hmr_to_eval/Turbopack CSR/1000 modules 23.35ms ± 0.67ms 23.02ms ± 0.82ms -1.40%
bench_startup/Turbopack CSR/1000 modules 1114.34ms ± 11.59ms 1107.81ms ± 17.32ms -0.59%

@mehulkar
Copy link
Contributor Author

The Turborepo Rust testing on macOS is failing but is fixed in #6517

@mehulkar mehulkar merged commit 1431566 into main Nov 20, 2023
52 of 66 checks passed
@mehulkar mehulkar deleted the mk/windows branch November 20, 2023 21:29
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