-
Notifications
You must be signed in to change notification settings - Fork 50
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
Fix windows support #323
Conversation
e19039e
to
ed2235e
Compare
c785178
to
f6f0aa0
Compare
@rolandtritsch @sksamuel The PR is ready albeit there is one issue (mentioned in description). |
f6f0aa0
to
f028af4
Compare
f028af4
to
798f981
Compare
Hi Matthew. This week I am on vacation. Will take a look at this next week.
- RT
…On Mon, Sep 16, 2024 at 3:40 PM Matthew de Detrich ***@***.***> wrote:
@rolandtritsch <https://github.com/rolandtritsch> The PR is ready albeit
there is one issue (mentioned in description).
—
Reply to this email directly, view it on GitHub
<#323 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFD5FMLBHF2LLT5Q67YSMLZW3UV7AVCNFSM6AAAAABOGVYEQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJTGEYTSNJYHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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
|
Hi Matthew. For now let's just remove the nmesos test.
…---
***@***.***/+353-86-8563976
Thumb-powered. Excuse the curtness.
On Tue 24 Sept 2024, 11:45 Matthew de Detrich, ***@***.***> wrote:
https://github.com/scoverage/sbt-coveralls/actions/runs/10886410611/job/30206347056?pr=323#step:6:118
Hi @mdedetrich <https://github.com/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
<https://github.com/NinesStack/nmesos/blob/trunk/src/test/scala/ninesstack/nmesos/cli/CliSpec.scala#L20-L25>
scripted sbt-test fails
<https://github.com/scoverage/sbt-coveralls/actions/runs/10886410611/job/30206347056?pr=323#step:6:199>
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.
—
Reply to this email directly, view it on GitHub
<#323 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFD5FKYMPNXID7AKFR57MDZYEYFJAVCNFSM6AAAAABOGVYEQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNZQG44TKMJXHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Sure thing, I'll do this on Sunday when I get back from vacation. |
@rolandtritsch I have removed the |
Hi @mdedetrich. Merged and published. Thanks for the fix. - https://github.com/scoverage/sbt-coveralls/releases/tag/v1.3.14 |
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 verificationThe changes that needed to be done to tests are fairly mechanical (largely involving handling Windows file separators) with notable changes listed below
generate.sh
/prepare.sh
) have been replaced with equivalent implementations in build.sbt sbt's dsl which provides us Windows portability for free.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.