Run some checks on windows #2730

cptartur · 2024-11-29T12:43:17Z

Introduced changes

Checklist

ddoktorski

Should we also update the scheduled workflow? Perhaps it's worth considering running tests on Windows only there instead of on every CI.

.github/workflows/ci.yml

crates/forge-runner/src/lib.rs

crates/sncast/src/state/state_file.rs

scripts/install_devnet.ps1

franciszekjob · 2024-12-08T22:12:10Z

crates/forge/tests/e2e/io_operations.rs

-            "No such file or directory [..]"
+            "[..] file [..]"


Could we make this failure message dependent on platform (applies to other places)?

Actually, I'd do the opposite. We should wrap these errors so we don't output OS errors directly to the user. And specifically, we shouldn't assert specific os errors in tests.

But until we do so, maybe I can assert the os specific error here

Changed all of them to be OS dependent except for one assert that asserted the path scripts/.

franciszekjob · 2024-12-08T22:12:37Z

crates/forge/tests/e2e/running.rs

-                #[should_panic(expected: "No such file or directory (os error 2)")]
+                #[should_panic(expected: "file")]


Same as above

.github/workflows/ci.yml

franciszekjob · 2024-12-08T22:28:27Z

.github/workflows/ci.yml

+env:
+  DEVNET_TAG: "v0.2.2"
+


We should use commit hash because it is immutable, unlike a tag which is mutable therefore could theoretically pose some risk.

This still needs to be changed 😅

Updated here as well

crates/sncast/src/state/state_file.rs

franciszekjob · 2024-12-10T15:40:45Z

.github/workflows/ci.yml

+env:
+  DEVNET_TAG: "v0.2.2"
+


This still needs to be changed 😅

franciszekjob · 2024-12-10T15:47:16Z

scripts/install_devnet.ps1

Do we really need all these comments?

.github/workflows/ci.yml

Co-authored-by: Franciszek Job <[email protected]>

ddoktorski

I still think it would be better to test Windows only in the scheduled workflow. Testing it with every single change isn’t crucial and overall it will slow down our CI and make it harder to maintain.

.github/workflows/ci.yml

ddoktorski · 2024-12-11T09:45:17Z

Can we create an issue to investigate whether the cache action functions correctly, considering that the first CI run on a branch takes significantly longer than subsequent runs, and on Windows it is much longer than on Linux?

Example:
https://github.com/foundry-rs/starknet-foundry/actions/runs/12178894598/job/33969862907 <- first run does not find any cache
https://github.com/foundry-rs/starknet-foundry/actions/runs/12179972960/job/33973292419

cptartur · 2024-12-13T11:57:08Z

Can we create an issue to investigate whether the cache action functions correctly, considering that the first CI run on a branch takes significantly longer than subsequent runs, and on Windows it is much longer than on Linux?

Example: https://github.com/foundry-rs/starknet-foundry/actions/runs/12178894598/job/33969862907 <- first run does not find any cache https://github.com/foundry-rs/starknet-foundry/actions/runs/12179972960/job/33973292419

@ddoktorski I'd just merge this and we'll see if te build times are problematic. And if so, we can disable these checks at any time.

# Conflicts: # CHANGELOG.md # Cargo.toml

kkawula · 2024-12-17T15:24:15Z

crates/forge/tests/e2e/mod.rs

+#[cfg(not(target_os = "windows"))]
 mod fork_warning;
+#[cfg(not(target_os = "windows"))]


Is there any issue that addresses the removal of these?

cptartur added 30 commits November 29, 2024 13:40

Run some checks on windows

ea31dd2

Remove linux specific expected message

3fb3906

Run 2.8.3 only on ubuntu-latest

5ebc63a

Ignore fork related integration tests on windows

b738f12

Ignore fork related integration tests on windows

a7e4b4b

Do not run coverage and profiler tests on windows

864818a

Add install_devnet.ps1

6c7cb45

Run install_devnet.ps1 on windows

72fd826

Fix nextest platform specific uploads

f4e15ea

Always run step in bash

527325b

Change path on windows

cccb52b

Fix path to platform

49b9ab4

Fix tag name in install_devnet.ps1

75dfa7d

Du not run forking e2e tests in windows

3a849d8

Add --no-fail-fast to forge e2e tests

462d546

Add different installation steps for windows and Linux/macOS

31ec41f

Always run e2e prep steps using bash

198ba82

Disable setup_fork::incompatible_abi on windows

0221ce3

Merge branch 'master' into 2716-run-tests-on-windows

a6a9b5e

Disable cheat_fork::store_load_cairo0_contract on windows

97b7eb0

System specific devnet path

1945681

System specific devnet killing

45c531d

Fix windows devnet path

5c9100c

Remove os specific error from test_invalid_private_key_file_path

e3f49d9

Remove os specific error from test_directory_non_existent

6c73d27

Handle .exe in sncast name in test_no_output_path_specified

0a3384d

Remove os specific error from test_invalid_path

ee04138

Remove os specific path from test_initialized_script_compiles

6707dc7

Use different scripts depending on the os

ea8c9af

Use powershell on windows

988d6d7

cptartur marked this pull request as ready for review December 5, 2024 10:28

cptartur requested review from ddoktorski, kkawula and franciszekjob as code owners December 5, 2024 10:28

cptartur added 2 commits December 5, 2024 11:56

Run profiler only on not windows

84f40bf

Remove os specific asserts

409eeab

ddoktorski reviewed Dec 5, 2024

View reviewed changes

franciszekjob requested changes Dec 8, 2024

View reviewed changes

cptartur added 4 commits December 9, 2024 17:00

Remove unnecessary profiler installation

34178da

Mention trace file changes in CHANGELOG.md

92c025b

Remove unnecessary todo

833320c

Use revision instead of tag

1a51b13

cptartur requested review from ddoktorski and franciszekjob December 9, 2024 16:10

cptartur added 4 commits December 10, 2024 14:55

Restore OS specific errors for files

e9e18ed

Lint, use reference

04637a9

Fix windows error

cbefce3

Fix windows error

e351f64

franciszekjob reviewed Dec 10, 2024

View reviewed changes

cptartur added 2 commits December 10, 2024 17:11

Use devnet revision on ci

0112d70

Remove comments

aa534d1

cptartur requested a review from franciszekjob December 10, 2024 16:11

franciszekjob approved these changes Dec 10, 2024

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

Update .github/workflows/ci.yml

91d8ec5

Co-authored-by: Franciszek Job <[email protected]>

ddoktorski approved these changes Dec 11, 2024

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

cptartur added 2 commits December 13, 2024 14:38

Remove unnecessary test step

8c209cc

Merge branch 'master' into 2716-run-tests-on-windows

f641187

# Conflicts: # CHANGELOG.md # Cargo.toml

kkawula reviewed Dec 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run some checks on windows #2730

Run some checks on windows #2730

cptartur commented Nov 29, 2024

ddoktorski left a comment

franciszekjob Dec 8, 2024

cptartur Dec 9, 2024

cptartur Dec 9, 2024

cptartur Dec 10, 2024

franciszekjob Dec 8, 2024

franciszekjob Dec 8, 2024

franciszekjob Dec 10, 2024

cptartur Dec 10, 2024

franciszekjob Dec 10, 2024

franciszekjob Dec 10, 2024

cptartur Dec 10, 2024

ddoktorski left a comment

ddoktorski commented Dec 11, 2024

cptartur commented Dec 13, 2024

kkawula Dec 17, 2024

		#[should_panic(expected: "No such file or directory (os error 2)")]
		#[should_panic(expected: "file")]

Run some checks on windows #2730

Are you sure you want to change the base?

Run some checks on windows #2730

Conversation

cptartur commented Nov 29, 2024

Introduced changes

Checklist

ddoktorski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddoktorski left a comment

Choose a reason for hiding this comment

ddoktorski commented Dec 11, 2024

cptartur commented Dec 13, 2024

Choose a reason for hiding this comment