Skip to content
This repository was archived by the owner on Apr 14, 2021. It is now read-only.

Windows CI: Azure Pipelines#6899

Merged
8 commits merged intorubygems:masterfrom
janpio:cross_platform_ci
Feb 15, 2019
Merged

Windows CI: Azure Pipelines#6899
8 commits merged intorubygems:masterfrom
janpio:cross_platform_ci

Conversation

@janpio
Copy link
Copy Markdown
Contributor

@janpio janpio commented Jan 9, 2019

What was the end-user problem that led to this PR?

When I wanted to start to work on #6841 by adding a few tests, I quickly learned that bundler's test suite does not pass on Windows: #6868

What was your diagnosis of the problem?

I decided to start working on fixing this, and created a few issues already: https://github.com/bundler/bundler/search?q=%22%5BWindows+test+failure%5D%22&unscoped_q=%22%5BWindows+test+failure%5D%22&type=Issues

But it become clear to me, that the only way to really track the improvement was to add CI for Windows - otherwise opening PRs with improvements would not really make sense.

What is your fix for the problem, implemented in this PR?

This PR first makes it possible to run the test suite on Windows at all by "hiding" a bit of code behind Gem.win_platform? (Spec::Manpages.setup made it super complicated to run on Windows - and is not really necessary to run the test suite).

It also works around a strange bug concerning thor's (a vendored dependency) use of readline which crashes in strange ways on Windows (see #6902). And it adds applies a patch to the readline implementation used on CI (via RubyInstaller ruby) that otherwise blocks the tests (see #6907).

Then it adds CI configuration for Azure Pipelines, a free for OSS CI platform. I configured the Azure Pipelines build to create a JUnit compatible report if the test suite and display it in the "Tests" tab of the build.

Why did you choose this fix out of the possible options?

I think disabling this code for Windows is a valid choice to get the ball rolling, later this will undone and the underlying problem will be fixed.

I chose the workarounds for #6902 and #6907 as I couldn't find a proper bugfix yet - so this will be one of the things to be fixed using Windows CI.

Azure Pipelines is a great CI platform for Windows (and later even multiple platforms if we choose to also run the tests on Linux and macOS).

I chose not to adapt the rake travis task (yet) because it is much more complex than the "simple" bin/rspec test suite run which I could directly execute in the CI scripts of Azure Pipelines.

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Jan 10, 2019

Why should we use Azure Pipeline? Its configuration is out of scope for Windows support.

@janpio
Copy link
Copy Markdown
Contributor Author

janpio commented Jan 10, 2019

Because AppVeyor has much more strict limits on parallel builds (for OSS projects), slower build times and less/other functionality. Right now this doesn't matter that much as the tests are only run for 1 Ruby, but if you want to get to a similar setup to Travis (many Rubies tested), this will get very relevant.

Any reason why you think Azure Pipelines is not a good idea?

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Jan 10, 2019

Any reason why you think Azure Pipelines is not a good idea?

Cost of Maintainance. I'm okay to use Travis and Appveyor only. I'm against to add the new CI platform that was duplicated process with existence CI.

@janpio
Copy link
Copy Markdown
Contributor Author

janpio commented Jan 10, 2019

Then I suggest removing AppVeyor from this PR and only add the Azure Pipelines configuration. It will enable me much better to fix the Windows test failures as I get feedback on all 3 relevant platforms at the same time, in much less time.

Agree?

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Jan 10, 2019

Agree?

Don't agree. If you enabled only Windows, I will accept to merge this PR. It's better to make the small start at first.

@janpio janpio changed the title Windows (and cross platform) CI: AppVeyor + Azure Pipelines Windows CI: Azure Pipelines Jan 10, 2019
@janpio
Copy link
Copy Markdown
Contributor Author

janpio commented Jan 10, 2019

Ok.

I removed AppVeyor and the Linux and macOS builds on Azure Pipelines and updated the initial PR description to match this stripped down PR.

(This also makes it unnecessary to fix stuff like #6900, so this is good to merge right now. 🚀)

@janpio
Copy link
Copy Markdown
Contributor Author

janpio commented Jan 10, 2019

To pull it out of the discussion on a code line at https://github.com/bundler/bundler/pull/6899/files/e182fcbeaa564f0d06a3a3d7fa62e3d8e132a873#diff-501fea893f314bd6414dabb1718c8127:

This PR is currently on hold until I figure out what is going on with readline.
I will report back.

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Jan 11, 2019

@janpio Thanks. I'm not familiar with Azure Pipeline yet. It's a good time to learn it for me. I also help to prepare to support the Windows environment.

@janpio janpio force-pushed the cross_platform_ci branch 2 times, most recently from 79e5f62 to f26a978 Compare January 11, 2019 19:53
@janpio janpio force-pushed the cross_platform_ci branch 3 times, most recently from dd27518 to 909c37f Compare January 14, 2019 21:12
@janpio janpio force-pushed the cross_platform_ci branch from 909c37f to b51b2be Compare January 14, 2019 22:01
@janpio
Copy link
Copy Markdown
Contributor Author

janpio commented Jan 14, 2019

Azure Pipelines can be pretty awesome - short wait times for builds and ok build times as well. And loads of different functionality if you need it.


This PR is now good to merge as I found a workaround for #6902 that does not require any code changes in vendored thor! Also #6907 has a workaround implemented in the CI script as well (thanks to @deivid-rodriguez!):

https://dev.azure.com/janpio/bundler/_build/results?buildId=606
https://dev.azure.com/janpio/bundler/_build/results?buildId=606&view=ms.vss-test-web.test-result-details


To activate Azure Pipelines for bundler/bundler, one of the maintainers (@hsbt maybe?) should follow these steps: https://samcogan.com/free-azure-build-pipelines-for-open-source-projects/

(For the template step, there should be a simple ruby template or just select an empty template - the actual build configuration is part of this PR and will come from here. No manual configuration should be necessary at all.)

Then we should be able to start a build with the configuration of this PR by closing and reopening the PR or just merge this into master and see the first build on master (that this PR creates a good configuration can be seen at https://dev.azure.com/janpio/bundler/_build/results?buildId=606).

(Ping me on Slack, username janpio, if you need any help with activating Azure Pipelines.)

displayName: 'ruby bin/rake spec:deps'

- script: |
ruby bin/rspec --format progress -r rspec_junit_formatter --format RspecJunitFormatter -o rspec/bundler-junit-results.xml
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this looks like it won't be testing against our chosen RubyGems versions, which is what the travis tasks handle

Copy link
Copy Markdown
Contributor Author

@janpio janpio Jan 27, 2019

Choose a reason for hiding this comment

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

Yes, I decided to not include this complexity already when there are >600 failing tests on Windows. From the PR description:

I chose not to adapt the rake travis task (yet) because it is much more complex than the "simple" bin/rspec test suite run which I could directly execute in the CI scripts of Azure Pipelines.

My idea was to start small, fix a few hundred failing tests, and then add the more complex build setup like you have it on Travis when the env differences will actually be visible and not hidden behind 600 test failures ;)

I would prefer to go forward with this much simpler build on Azure Pipelines, and revisit this later. Agree?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I also think it's fine to start small and get bundler CI passing against any rubygems version, and later take care about completing the matrix and make sure bundler CI passes against all rubygems versions.

@colby-swandale
Copy link
Copy Markdown
Member

When Azure announced Ruby support for their Azure CI service, i was quick to jump onboard and looked into seeing if we could use it ourselves. But thinking about it over the past few weeks, i've been wondering if we're better off using AppVeyour instead.

Appveyor is being used by most OSS Ruby projects on Github at the moment, and a lot of those Appveyor pipelines are being maintained by @MSP-Greg, who is a semi-regular contributor to the RubyGems and Bundler projects. If there was ever an issue with testing Bundler on Windows, we would have someone that is quite familiar with both domains to help resolve any issues.

\cc @indirect thoughts?

@MSP-Greg
Copy link
Copy Markdown
Contributor

MSP-Greg commented Feb 9, 2019

@colby-swandale

I've been wondering if we're better off using AppVeyor instead.

When Azure Pipelines (AP) was first announced, I thought I'd check into building Ruby trunk there. AP was new, MSFT obviously has resources far beyond what Travis & Appveyor have, and most importantly, it would run a lot of jobs parallel.

I was trying code based on ruby-loco (largely PowerShell) and the ruby/ruby appveyor build code. A few notes:

  1. At the time, I had to install everything. A version of Ruby (you need Ruby to build Ruby), the full MSYS2/MinGW build system, etc. The code isn't brittle, but it's not something a non-windows user would want to jump into.

  2. Both Appveyor (AV) & Travis 'hold' environment variables as one moves between pre-defined script steps. AP does not. So there's a bit of monkeying around with using a AP specific API to share them between script steps.

  3. There were a lot of other quirky things, like logs showing absolute time, not relative, etc. I even had to build a special package of 7-zip, since the normal windows installer had an issue. At the time, there was not a lot of pre-installed software, at least compared to AV.

Form bouncing around encouraging and helping with AV tests, the main issue I come across is the 'it takes too long'. AP will help with that. But, if I ported some repos over to AP, and the script doubled or tripled in size, most people would run away very quickly, as it's just too different from Travis.

I'd certainly like to see bundler testing on Windows. I looked at it a long time ago, now I don't recall the difference between the 'bundler' testing in RG, vs the testing here. I haven't really looked at this PR, nor what work @janpio and @hsbt may have done.

Off topic:

I think many people in the OSS community don't like seeing a big guy (MSFT) rolling over a small guy (AV). AV has always been receptive to changes, but it takes forever to update their images (Ruby 2.6 isn't available yet), and they don't allow most accounts to test parallel (although Ruby CI runs two or three). At one point I posted several suggestions for AP, and several MSFT people were participating. They are supporting AP and they do want to see it adopted by the OSS community....

Sorry for the long post. I'm interesting in helping with whatever.

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

I support trying out Azure Pipelines and see how it goes. We have a volunteer, @janpio, who has already done this setup and was (hopefully, still is) willing to fix the test suite on Windows after this. I volunteer to set this up on the bundler organization so @janpio can proceed with his work!

@indirect
Copy link
Copy Markdown

I'm also happy to try out Azure Pipelines, since it seems like it will help us with our goal of running the Bundler tests on Windows. Once we have passing tests, we can compare Appveyor and Azure and see which one meets our needs best in the long run.

I've added bundler/bundler on GitHub to bundler/bundler on Azure.

@janpio
Copy link
Copy Markdown
Contributor Author

janpio commented Feb 10, 2019

Thanks for adding the Azure Pipelines integration @indirect.

As I removed the rspec_junit_formatter dependency (see the ongoing discussion about that), Azure Pipelines picked up the commit and ran it - and gave this PR a big red "X".

The Azure Pipelines tests of course will fail until I have all the Windows bugs fixes merged, which can take quite some time I suppose. Should I investigate if it is possible to configure Azure Pipelines to report "success" although the tests are failing?

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

Maybe if you add || exit 0 for now after the command that runs the specs, it will have that effect for now. We can remove it after we manage to get it green.

@janpio
Copy link
Copy Markdown
Contributor Author

janpio commented Feb 10, 2019

That worked @deivid-rodriguez ❤️

Only the rspec_junit_formatter problem/suggestion left: https://github.com/bundler/bundler/pull/6899/files/485bbf1e2b32ed603dbd0c790adf35607b8c9ac5#diff-6ac292c8e0d03698a046322989f7e57c

@indirect
Copy link
Copy Markdown

@janpio does the version of the gem matter? When you put it in the gemspec, you said ~> 0.2.3, which means >= 0.2.0 && < 0.3.0. When you run gem install rspec_junit_formatter, it seems to be installing version 0.4.1.

On my machine, I was able to make it work by re-ordering the require. The script is currently running this command:

ruby bin/rspec --format progress -r rspec_junit_formatter --format RspecJunitFormatter -o rspec/bundler-junit-results.xml 

When I changed it to this command instead, it seemed to work:

ruby -r rspec_junit_formatter bin/rspec --format progress --format RspecJunitFormatter -o rspec/bundler-junit-results.xml 

Maybe give that a shot?

@janpio
Copy link
Copy Markdown
Contributor Author

janpio commented Feb 11, 2019

does the version of the gem matter?

Nope, that was just a copy paste thing from fastlane where we seem to be using that version.

When I changed it to this command instead, it seemed to wor

That did indeed work ❤️ But why?

@indirect
Copy link
Copy Markdown

indirect commented Feb 11, 2019 via email

@janpio
Copy link
Copy Markdown
Contributor Author

janpio commented Feb 11, 2019

Understood, it's always in the details ;)


And that's a wrap, everything is working as it should now (I think):

image
https://dev.azure.com/bundler/bundler/_build/results?buildId=9&view=ms.vss-test-web.build-test-results-tab
https://dev.azure.com/bundler/bundler/_build/results?buildId=9

I cleaned up the commits a bit, so this is good to merge now 🚀
Thanks everyone that gave feedback and helped me finish this.

@janpio
Copy link
Copy Markdown
Contributor Author

janpio commented Feb 14, 2019

Maybe you noticed that the "bundler.bundler" check is still in status "In progress - This check has started..." although on Azure Pipelines it has finished after ~40 minutes just fine. This seems to be a problem on Azure Pipelines caused by the numbers of tests failures of our build. I am in contact with their development team, who are working on a fix right now. Will update when there is news on this.

@janpio janpio force-pushed the cross_platform_ci branch 2 times, most recently from a0e48c2 to 8ace8a5 Compare February 15, 2019 17:26
@ethomson
Copy link
Copy Markdown

👋 Apologies for the problem with the check not getting updated from Azure Pipelines. We had a bug in the check updater code when making a call to our Test Case Manager API. We sorted that out and we think that things should be 👍 from now on.

@janpio
Copy link
Copy Markdown
Contributor Author

janpio commented Feb 15, 2019

Thanks @ethomson for the support with this, the Azure Pipelines check is indeed green now and this PR is finally really ready to merge 🚀

(I so want to work on decreasing that "671 failing tests" number!)

@colby-swandale
Copy link
Copy Markdown
Member

@bundlerbot r+

ghost pushed a commit that referenced this pull request Feb 15, 2019
6899: Windows CI: Azure Pipelines r=colby-swandale a=janpio

### What was the end-user problem that led to this PR?

When I wanted to start to work on #6841 by adding a few tests, I quickly learned that bundler's test suite does not pass on Windows: #6868

### What was your diagnosis of the problem?

I decided to start working on fixing this, and created a few issues already: https://github.com/bundler/bundler/search?q=%22%5BWindows+test+failure%5D%22&unscoped_q=%22%5BWindows+test+failure%5D%22&type=Issues 

But it become clear to me, that the only way to really track the improvement was to add CI for Windows - otherwise opening PRs with improvements would not really make sense.

### What is your fix for the problem, implemented in this PR?

This PR first makes it possible to run the test suite on Windows at all by "hiding" a bit of code behind `Gem.win_platform?` (`Spec::Manpages.setup` made it super complicated to run on Windows - and is not really necessary to run the test suite). 

It also works around a strange bug concerning `thor`'s (a vendored dependency) use of `readline` which crashes in strange ways on Windows (see #6902). And it adds applies a patch to the `readline` implementation used on CI (via RubyInstaller ruby) that otherwise blocks the tests (see #6907).

Then it adds CI configuration for Azure Pipelines, a free for OSS CI platform. I configured the Azure Pipelines build to create a JUnit compatible report if the test suite and display it in the "Tests" tab of the build.

### Why did you choose this fix out of the possible options?

I think disabling this code for Windows is a valid choice to get the ball rolling, later this will undone and the underlying problem will be fixed.

I chose the workarounds for #6902 and #6907 as I couldn't find a proper bugfix yet - so this will be one of the things to be fixed using Windows CI.

Azure Pipelines is a great CI platform for Windows (and later even multiple platforms if we choose to also run the tests on Linux and macOS).

I chose not to adapt the `rake travis` task (yet) because it is _much_ more complex than the "simple" `bin/rspec` test suite run which I could directly execute in the CI scripts of Azure Pipelines.


Co-authored-by: Jan Piotrowski <piotrowski+git@gmail.com>
Co-authored-by: Jan Piotrowski <piotrowski+github@gmail.com>
@ghost
Copy link
Copy Markdown

ghost commented Feb 15, 2019

Build succeeded

@ghost ghost merged commit 8ace8a5 into rubygems:master Feb 15, 2019
This pull request was closed.
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.

9 participants