Skip to content
This repository was archived by the owner on Dec 7, 2019. It is now read-only.

Conversation

@Nilzor
Copy link
Contributor

@Nilzor Nilzor commented Dec 3, 2017

Ensure no tests fail due to \n differs to \r\n on Windows,
while maintaining Linux/MacOS compatibility

edit: Might hold off the merge until I've verified that tests in HtmlReportSpec passes. I'm struggling to generate app.min.js (or you can verify it on AppVeyor)

edit 2: HtmlReportSpec have other issues not related to Linefeed or Windows. I believe this PR can be merged

Ensure no tests fail due to \n differs to \r\n on Windows,
while maintaining Linux/MacOS compatibility
Template file is stored with \n and will not be affected
by operating system
@Nilzor
Copy link
Contributor Author

Nilzor commented Dec 3, 2017

Appveyor result: https://ci.appveyor.com/project/Nilzor/composer/build/appveyor%204#L454

Down to 6 errors - all related to HtmlReportSpec.kt, which requires a lot more work to fix on Windows (Not linefeed-related)

Copy link
Contributor

@yunikkk yunikkk left a comment

Choose a reason for hiding this comment

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

Sorry for the long review.

@yunikkk yunikkk merged commit 6e039df into gojuno:master Jan 22, 2018
@yunikkk
Copy link
Contributor

yunikkk commented Jan 22, 2018

@Nilzor regarding 6 issues with the HtmlReportSpec.kt - tests expect Nodejs is present and are supposed to work in Docker container with ci/build.sh, which contains all the needed environment for that.
I guess we'll need a separate script and container for running all on Windows (or is running Linux Docker images possible/not painful on Windows, I'm not sure really).

@Nilzor
Copy link
Contributor Author

Nilzor commented Jan 23, 2018

Do we really need to be able to run the tests on Windows though? Having the production code running on Windows and having the development environment run are two different things.

Running docker on Windows is definitely possible, I'm just not sure it's worth investing in it. There are a couple of approaches we could take:

  • Create a Windows version of build.sh : build.ps1 or build.cmd (most amount of work).
  • Document how to run Docker in Bash on Ubuntu for Windows and make any changes needed, if any (I haven't tried that approach yet - not sure if Docker will run in this environment)
  • Document how to run Docker in Linux VM on Windows (Basically not supporting Windows as a unit testing enviornment)

I'm leaning towards number 3, or not doing anything at all with regards to testing on Windows

@Nilzor
Copy link
Contributor Author

Nilzor commented Jan 23, 2018

PS: Thanks for the merge - could you also please make a new release build?

@yunikkk
Copy link
Contributor

yunikkk commented Jan 24, 2018

Well, running all tests on Windows would make us sure that all stuff continues working on Windows after new changes.
Let's stick with option 3 for now, ideally considering 1 one somewhere in future)

@yunikkk
Copy link
Contributor

yunikkk commented Jan 24, 2018

Pushed new release, too.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants