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

chore: Fix broken test on Windows #10900

Merged
merged 3 commits into from
Jun 10, 2021

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Jun 8, 2021

cat does not exist by default on Windows. GitHub actions adds a lot of stuff to the Windows runner to make it work, which is why the CI is ok (I'm often surprised at what works in there).

PathBuf::from(concat!(env!("CARGO_MANIFEST_DIR")))
.parent()
.unwrap()
.to_path_buf()
Copy link
Member Author

@dsherret dsherret Jun 8, 2021

Choose a reason for hiding this comment

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

This just makes the paths that get outputted while the tests are running appear resolved.

cmd: ["cat", "089_run_allow_list.ts"],
cmd: Deno.build.os === "windows"
? ["cmd", "/c", "type", "089_run_allow_list.ts"]
: ["cat", "089_run_allow_list.ts"],
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't like doing these conditionals. Let me know if you can think of something that exists on all three operating systems by default.

Copy link
Member

Choose a reason for hiding this comment

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

curl exists on all three now AFAIK. Or maybe echo?

Copy link
Member Author

@dsherret dsherret Jun 8, 2021

Choose a reason for hiding this comment

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

Oh, echo is good. Thanks!

Edit: Oh, echo isn't an executable, so curl it is! Should be ok for most windows developers (edit: yeah, the edition that introduced curl already reached EOL https://docs.microsoft.com/en-us/windows/release-health/status-windows-10-1803).

Should be ok because last Windows 10 version that was released without curl is now EOL as of last month.
@dsherret dsherret merged commit 9d706d7 into denoland:main Jun 10, 2021
@dsherret dsherret deleted the tests-fix-for-windows branch June 10, 2021 16:35
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.

None yet

2 participants