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 windows support #323

Merged
merged 3 commits into from
Sep 30, 2024
Merged

Conversation

mdedetrich
Copy link
Contributor

@mdedetrich mdedetrich commented Sep 14, 2024

Resolves: #294

This PR makes coveralls works under windows, it consists of 2 commits

  • Make tests portable so it runs on Windows: Various changes to tests so that they can run properly on Windows (but no changes to core codebase)
  • Add windows support by removing POSIX line seperator normalization: Actual changes to core codebase so that sbt-coveralls can run under windows plus adding windows to github actions CI for verification

The changes that needed to be done to tests are fairly mechanical (largely involving handling Windows file separators) with notable changes listed below

  • Shell scripts (generate.sh/prepare.sh) have been replaced with equivalent implementations in build.sbt sbt's dsl which provides us Windows portability for free
  • Windows equivalent xml template files (i.e. .xml.template) has been introduced as .xml.windows.template. The only difference is using \ instead of /, there might have been smarter solutions to this but I kept it this way to keep it simple.

In regards to the actual fix, this involved removing the so called normilzations which involved replacing the Windows file separator with the POSIX one. I am not sure what the original intent of these "normalizations" were, but making all representations/serializations of paths consistently use \ under Windows (and / under POSIX although this behaviour is unchanges) fixes all of the issues.

The only current issue is that the ninesstack.nmesos.cli.CliSpec scripted sbt-test fails because this external scala project itself doesn't appear to be portable on Windows. I don't know what the best course of action here is as that repo is external to this one, simplest solution might be to just update that project so that the test passes under windows which will automatically fix it here.

@mdedetrich mdedetrich marked this pull request as draft September 14, 2024 09:15
@mdedetrich mdedetrich force-pushed the fix-windows-support branch 29 times, most recently from e19039e to ed2235e Compare September 16, 2024 12:13
@mdedetrich mdedetrich force-pushed the fix-windows-support branch 14 times, most recently from c785178 to f6f0aa0 Compare September 16, 2024 14:25
@mdedetrich mdedetrich marked this pull request as ready for review September 16, 2024 14:26
@mdedetrich
Copy link
Contributor Author

mdedetrich commented Sep 16, 2024

@rolandtritsch @sksamuel The PR is ready albeit there is one issue (mentioned in description).

@rolandtritsch
Copy link
Member

rolandtritsch commented Sep 17, 2024 via email

@rolandtritsch
Copy link
Member

https://github.com/scoverage/sbt-coveralls/actions/runs/10886410611/job/30206347056?pr=323#step:6:118

Hi @mdedetrich. The windows test fails. Any idea what might be wrong here?

@mdedetrich
Copy link
Contributor Author

https://github.com/scoverage/sbt-coveralls/actions/runs/10886410611/job/30206347056?pr=323#step:6:118

Hi @mdedetrich. The windows test fails. Any idea what might be wrong here?

Yes, I said so why in the original post of the PR

The only current issue is that the ninesstack.nmesos.cli.CliSpec scripted sbt-test fails because this external scala project itself doesn't appear to be portable on Windows. I don't know what the best course of action here is as that repo is external to this one, simplest solution might be to just update that project so that the test passes under windows which will automatically fix it here.

@rolandtritsch
Copy link
Member

rolandtritsch commented Sep 25, 2024 via email

@mdedetrich
Copy link
Contributor Author

Hi Matthew. For now let's just remove the nmesos test.

Sure thing, I'll do this on Sunday when I get back from vacation.

@mdedetrich
Copy link
Contributor Author

@rolandtritsch I have removed the nmesos test as instructed and everything is passing now

@rolandtritsch rolandtritsch merged commit 3fc2fa6 into scoverage:main Sep 30, 2024
4 checks passed
@rolandtritsch
Copy link
Member

Hi @mdedetrich. Merged and published. Thanks for the fix. - https://github.com/scoverage/sbt-coveralls/releases/tag/v1.3.14

@mdedetrich mdedetrich deleted the fix-windows-support branch September 30, 2024 08:25
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.

sbt-coveralls failing on windows-latest github actions image
2 participants