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 tests (CI) #59

Closed
3 tasks done
Tapppi opened this issue Jun 18, 2016 · 13 comments
Closed
3 tasks done

Fix windows tests (CI) #59

Tapppi opened this issue Jun 18, 2016 · 13 comments

Comments

@Tapppi
Copy link
Member

Tapppi commented Jun 18, 2016

Ok, not sure how I managed to miss this before. Tests with mocked git don't work on windows due to mock-git and its dep mock-bin:

Both of these packages will need updates to make the mock tests work. I have created PRs in the respective repos and verified that standard-version tests are fixed with them. This issue is to track the progress of those fixes.

@Tapppi
Copy link
Member Author

Tapppi commented Jun 23, 2016

All issues resolved, just waiting on a mock-git release.

@bcoe
Copy link
Member

bcoe commented Jun 23, 2016

@Tapppi digging into the AppVeyor stuff, I'm pretty stumped. A few things I noticed:

  • there's a CI variable set to true, I wonder if this is somehow getting picked up in the formatting string.
  • running "%cd --date=iso" causes an exception, unknown date format ISO. As %ci is supposed to be iso format itself, is this causing the issue?

CC: @nexdrew

@Tapppi
Copy link
Member Author

Tapppi commented Jun 23, 2016

@bcoe, I'm gonna sum the CI shenanigans here since it's been all over the place now. @nexdrew 's comment in the other PR had everything right, so:

  1. The whole Appveyor test is broken, but not affecting use or tests on Windows locally. @bcoe, as you point out, the CI env var is exactly the thing breaking the build. The var can't be removed and thus the formatting string (with the %ci% env var reference) needs to be changed. Explanation can be found in @nexdrew 's comment and above it in mine. I'll create PRs in conventional-changelog-core tonight, unless someone can do it before me, for
    • Appveyor CI config
    • Adding a space after %ci in the format string to fix this
  2. The mock-git tests dont work, but will be fixed as soon as @stevemao releases a new version of mock-git with the fixes outlined in this issue's OP.

@bcoe @nexdrew @stevemao does this clear this stuff up finally? That should be a good push forward IMO.

Also, @nexdrew to answer your question of what blocks Windows usage: Nothing! 😊 I'm using this package daily on Windows.

@stevemao
Copy link
Member

mock-git should work on Windows now.

@Tapppi Tapppi changed the title mock-git tests don't work on windows Fix windows tests (CI) Jun 24, 2016
@Tapppi
Copy link
Member Author

Tapppi commented Jun 24, 2016

Closed #57 and renamed this to reflect the fact that, at least IMO, this is the best place to track the Appveyor CI issues for now.

@Tapppi
Copy link
Member Author

Tapppi commented Jun 24, 2016

Opened PRs for appveyor config on conventional-changelog-core and git-raw-commits. Also opened windows fix PR in git-raw-commits: conventional-changelog-archived-repos/git-raw-commits#10. It's the most logical place, since it will actually fix all percent sign variable expansion problems by escaping them on windows.

@bcoe
Copy link
Member

bcoe commented Jun 24, 2016

@Tapppi this agrees with what I was seeing, moving forward with fixes seems smart to me ... I don't think any of us want to spin up Windows VMs too frequently, having AppVeyor in place to stop regressions will be nice.

@Tapppi
Copy link
Member Author

Tapppi commented Jun 26, 2016

So unfortunately my previous fix to git-raw-commits was a non-fix.. Opened a new PR with an actual fix and tests to confirm it...

@Tapppi
Copy link
Member Author

Tapppi commented Jun 27, 2016

All but node 0.10 work now, with git-raw-commits v1.1.2. Don't have time right now to delve into that..

https://ci.appveyor.com/project/Tapppi/standard-version/build/29

@bcoe
Copy link
Member

bcoe commented Jun 27, 2016

@Tapppi I'm pretty comfortable with removing Node 0.10 from Windows integration tests, if you don't have time to dig into this further ... gets us to a much better place than where we were.

@Tapppi
Copy link
Member Author

Tapppi commented Jun 29, 2016

Yeah I think that's the best move forwards.

@bcoe
Copy link
Member

bcoe commented Jul 3, 2016

@Tapppi great work!

@bcoe bcoe closed this as completed Jul 3, 2016
@stevemao
Copy link
Member

stevemao commented Jul 3, 2016

Thanks @Tapppi!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants