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

[WIP] [Rebased] Improve AppVeyor build #567

Closed
wants to merge 24 commits into from

Conversation

xtrasimplicity
Copy link
Member

@xtrasimplicity xtrasimplicity commented May 14, 2018

Summary

This is simply #505 rebased onto master. I didn't want to force push to improve-appveyor-build. ;)

Details

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (cleanup of codebase without changing any existing functionality)
  • Update documentation

Checklist:

  • I've added tests for my code
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

This change is Reviewable

mvz added 19 commits May 14, 2018 18:32
- The fat binaries for the ffi gem do not support Ruby 1.9.3, hence
  testing on this version on Windows makes no sense.
- To set Ruby version 2.0.0, the micro version also needs to be set.
The existing default is not needed, since ENV is always used as a base
for the local environment.
- Use inject on all versions of Ruby, not need to make a distinction
  here.
- Clean up overrides in WindowsEnvironmentVariables
- Override to_h in WindowsEnvironmentVariables so it correctly upcases
  ENV values.
SpawnProcess should work without specifying these, and there are no
specs that essentially override these defaults.
Instead of setting up or using a shell script, just use Ruby with a
script on the command line.
This splits the specs in a Unix and Windows versions, just like for the
specs for commands with spaces.
This method basically returns one of the initializer arguments so makes
little sense.
@xtrasimplicity xtrasimplicity changed the title [Rebased] Improve AppVeyor build [Rebased] [WIP] Improve AppVeyor build May 14, 2018
@xtrasimplicity xtrasimplicity changed the title [Rebased] [WIP] Improve AppVeyor build [WIP] [Rebased] Improve AppVeyor build May 14, 2018
@xtrasimplicity
Copy link
Member Author

xtrasimplicity commented May 14, 2018

Just a couple of observations so far.
One of the examples fails with:

 Given I successfully run `aruba-test-cli` # C:/projects/aruba/lib/aruba/cucumber/command.rb:10
      Command "aruba-test-cli" not found in PATH-variable "C:/projects/aruba/tmp/aruba/getting-started-app/bin;C:/projects/aruba/tmp/aruba/getting-started-app/exe;C:/Ruby200/lib/ruby/gems/2.0.0/bin;C:/projects/aruba/bin;C:/projects/aruba/exe;C:\Ruby200\bin;C:\Perl\site\bin;C:\Perl\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Program Files\7-Zip;C:\Program Files\Microsoft\Web Platform Installer\;C:\Tools\GitVersion;C:\Tools\PsTools;C:\Program Files\Git LFS;C:\Program Files (x86)\Subversion\bin;C:\Program Files\Microsoft SQL Server\120\Tools\Binn\;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\110\Tools\Binn\;C:\Program Files (x86)\Microsoft SQL Server\120\Tools\Binn\;C:\Program Files\Microsoft SQL Server\120\DTS\Binn\;C:\Program Files (x86)\Microsoft SQL Server\120\Tools\Binn\ManagementStudio\;C:\Tools\WebDriver;C:\Program Files (x86)\Microsoft SDKs\TypeScript\1.4\;C:\Program Files (x86)\Microsoft Visual Studio 12.0\Common7\IDE\PrivateAssemblies\;C:\Program Files (x86)\Microsoft SDKs\Azure\CLI\wbin;C:\Ruby193\bin;C:\Tools\NUnit\bin;C:\Tools\xUnit;C:\Tools\MSpec;C:\Tools\Coverity\bin;C:\Program Files (x86)\CMake\bin;C:\go\bin;C:\Program Files\Java\jdk1.8.0\bin;C:\Python27;C:\Program Files\nodejs;C:\Program Files (x86)\iojs;C:\Program Files\iojs;C:\Users\appveyor\AppData\Roaming\npm;C:\Program Files\Microsoft SQL Server\130\Tools\Binn\;C:\Program Files (x86)\MSBuild\14.0\Bin;C:\Tools\NuGet;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE\CommonExtensions\Microsoft\TestWindow;C:\Program Files\Microsoft DNX\Dnvm;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\130\Tools\Binn\;C:\Program Files (x86)\Microsoft SQL Server\130\Tools\Binn\;C:\Program Files (x86)\Microsoft SQL Server\130\DTS\Binn\;C:\Program Files\Microsoft SQL Server\130\DTS\Binn\;C:\Program Files (x86)\Microsoft SQL Server\110\DTS\Binn\;C:\Program Files (x86)\Microsoft SQL Server\120\DTS\Binn\;C:\Program Files (x86)\Apache\Maven\bin;C:\Python27\Scripts;C:\Tools\NUnit3;C:\Program Files\Mercurial\;C:\Program Files\LLVM\bin;C:\Program Files\dotnet\;C:\Tools\curl\bin;C:\Program Files\Amazon\AWSCLI\;C:\Program Files (x86)\Microsoft SQL Server\140\DTS\Binn\;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE\Extensions\Microsoft\SQLDB\DAC\140;C:\Tools\vcpkg;C:\Program Files (x86)\Microsoft SQL Server\140\Tools\Binn\;C:\Program Files\Microsoft SQL Server\140\Tools\Binn\;C:\Program Files\Microsoft SQL Server\140\DTS\Binn\;C:\Program Files\PowerShell\6.0.0\;C:\Program Files\erl9.2\bin;C:\Program Files (x86)\nodejs\;C:\Program Files\Git\cmd;C:\Program Files\Git\usr\bin;C:\Program Files (x86)\Yarn\bin\;C:\Program Files (x86)\NSIS;C:\Tools\Octopus;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;C:\ProgramData\chocolatey\bin;C:\Users\appveyor\AppData\Local\Yarn\bin;C:\Program Files\AppVeyor\BuildAgent\". (Aruba::LaunchError)
      C:/projects/aruba/lib/aruba/processes/spawn_process.rb:261:in `command_string'
      C:/projects/aruba/lib/aruba/processes/spawn_process.rb:74:in `start'
      C:/projects/aruba/lib/aruba/command.rb:71:in `start'
      C:/projects/aruba/lib/aruba/api/commands.rb:240:in `start_command'
      C:/projects/aruba/lib/aruba/api/commands.rb:167:in `run_command_and_stop'
      C:/projects/aruba/lib/aruba/cucumber/command.rb:12:in `/^I successfully run `(.*?)`(?: for up to ([\d.]+) seconds)?$/'
      C:/projects/aruba/lib/aruba/platforms/local_environment.rb:22:in `call'
      C:/projects/aruba/lib/aruba/platforms/unix_platform.rb:78:in `with_environment'
      C:/projects/aruba/lib/aruba/api/core.rb:186:in `with_environment'
      C:/projects/aruba/lib/aruba/cucumber/hooks.rb:7:in `block in <top (required)>'
      features/hello_aruba.feature:3:in `Given I successfully run `aruba-test-cli`'
    Then the output should contain:           # C:/projects/aruba/lib/aruba/cucumber/command.rb:155
      """
      Hello, Aruba!
      """

Interestingly, it looks like it's assuming we're in a Unix environment, even though we're on Windows.

  C:/projects/aruba/lib/aruba/platforms/local_environment.rb:22:in `call'
      C:/projects/aruba/lib/aruba/platforms/unix_platform.rb:78:in `with_environment'
      C:/projects/aruba/lib/aruba/api/core.rb:186:in `with_environment'

With the Command "aruba-test-cli" not found in PATH-variable errors - given the path has been set correctly, perhaps we could try running them through bundler? (i.e. bundle exec bin/<path_to_script>)

Edit: This script incidentally is unix-only, but some of the others should theoretically be executable on windows, when passed to ruby? (As the ruby interpreter will ignore the shebang)

@xtrasimplicity
Copy link
Member Author

xtrasimplicity commented May 14, 2018

RbConfig::CONFIG['host_os'] returns mingw32 when executed via a ruby script from cucumber. Printing this to the console before invoking cucumber returns 'windows'.
https://ci.appveyor.com/project/cucumberbdd/aruba/build/857

expected `C:/Ruby200/bin/ruby -e "puts RbConfig::CONFIG['host_os']"` to have output string includes: "windows"
but was:
   mingw32 (RSpec::Expectations::ExpectationNotMetError)
./lib/aruba/cucumber/command.rb:147:in `/^(?:the )?(output|stderr|stdout)(?: from "([^"]*)")? should( not)? contain( exactly)? "([^"]*)"$/'

I'll implement a temporary work-around and see how things go. :)

@xtrasimplicity xtrasimplicity force-pushed the improve-appveyor-build-2 branch 2 times, most recently from 5755165 to 8b441b9 Compare May 14, 2018 12:33
@mvz
Copy link
Contributor

mvz commented May 30, 2018

@xtrasimplicity thanks a lot for working on this!

Would you mind changing your Reviewable settings so it doesn't add the button for repositiories that don't use Reviewable?

@xtrasimplicity
Copy link
Member Author

@mvz All done. Sorry, I hadn't noticed it's been injecting that button into my PRs!

@mvz
Copy link
Contributor

mvz commented May 30, 2018

@xtrasimplicity Thanks!

@xtrasimplicity
Copy link
Member Author

Given Aslak prefers that we merge rather than rebase, I'm going to close this one down and work on the original PR.

@mvz
Copy link
Contributor

mvz commented Jun 10, 2018

The original PR can be rebased too, and this is what I usually do.

  • Are any of the commits you created here worth cherry-picking?
  • Do you have any reference for Aslak's preference?

@xtrasimplicity
Copy link
Member Author

xtrasimplicity commented Jun 10, 2018

I'm manually cherry-picking the necessary commits and adding them on an as-needed basis. Aslak's comments can be found here: cucumber/cucumber-ruby#1299 (comment)

Edit: I've merged master into the other PR.

@mvz
Copy link
Contributor

mvz commented Jun 10, 2018

Ah 🙂. There are actually quite a few things I don't agree with in the linked article.

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

Successfully merging this pull request may close these issues.

2 participants