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

Fix AssemblyTests on Windows #3288

Merged
merged 4 commits into from
Jul 24, 2024
Merged

Conversation

lefou
Copy link
Member

@lefou lefou commented Jul 23, 2024

Depends on #3285

Windows has an env var called PATHEXT whish is usually something like this: .COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC
Basically a list of extensions considered as executable.
So the file in Mill's case needs to be a .bat file.

Issue analyzed and fixed by @sake92

Fix #3283

Copy link
Member Author

@lefou lefou left a comment

Choose a reason for hiding this comment

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

@sake92 Thank you for the fix! Can you please format it and also bump the Mill version to 0.11.9 so we see it succeeds? Thank you

@sake92
Copy link
Contributor

sake92 commented Jul 23, 2024

@lefou where do I bump the mill version, in .config\mill-version ?

@lefou
Copy link
Member Author

lefou commented Jul 23, 2024

@lefou where do I bump the mill version, in .config\mill-version ?

Yes.

@sake92
Copy link
Contributor

sake92 commented Jul 23, 2024

Seems like scalalib tests are green now.
https://github.com/com-lihaoyi/mill/actions/runs/10060348007

@lefou
Copy link
Member Author

lefou commented Jul 23, 2024

Looks like there are some new unrelated test failures for Windows now. Since we now know, the assembly tests are fixed by this PR, we could remove the Mill bump, and merge this PR (and close the bounty, I guess).

.config/mill-version Outdated Show resolved Hide resolved
Co-authored-by: Tobias Roeser <le.petit.fou@web.de>
@sake92
Copy link
Contributor

sake92 commented Jul 23, 2024

Looks like there are some new unrelated test failures for Windows now. Since we now know, the assembly tests are fixed by this PR, we could remove the Mill bump, and merge this PR (and close the bounty, I guess).

I guess it would be fair to make the CI green first, before closing the bounty. 😁

@lefou
Copy link
Member Author

lefou commented Jul 23, 2024

Looks like there are some new unrelated test failures for Windows now. Since we now know, the assembly tests are fixed by this PR, we could remove the Mill bump, and merge this PR (and close the bounty, I guess).

I guess it would be fair to make the CI green first, before closing the bounty. 😁

Yeah. That was the idea by not bumping Mill to cause unrelated issues.

@lefou
Copy link
Member Author

lefou commented Jul 24, 2024

I think the other test failures are valid but unrelated. They probably went under the radar since our Windows CI wasn't fully functional before #3285 .

@sake92 Looks like you are on Windows, so any further help to fix these would be much appreciated. 🙏‍

@sake92
Copy link
Contributor

sake92 commented Jul 24, 2024

@lefou we can merge this one?
It is easier to fix them one by one I think.

@lefou lefou merged commit c74a3fa into com-lihaoyi:main Jul 24, 2024
36 of 39 checks passed
@lefou lefou added this to the 0.11.11 milestone Jul 24, 2024
@sake92 sake92 deleted the fix-windows-AssemblyTests branch July 24, 2024 08:40
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.

Windows CI seems to be broken on newer versions of Mill (500USD)
2 participants