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

Improve AppVeyor Build #505

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Improve AppVeyor Build #505

wants to merge 7 commits into from

Conversation

mvz
Copy link
Contributor

@mvz mvz commented Nov 7, 2017

Summary

Work towards a green build on AppVeyor

Details

  • Ensure Ruby version variables match actual versions
  • Get ffi to install on older Rubies The FFI gem for mingw32 does not support 1.9.3.
  • Fix real test failures

Motivation and Context

AppVeyor builds always fail and hence are worse than useless right now.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

@mvz mvz force-pushed the improve-appveyor-build branch 3 times, most recently from 5491c13 to 8656962 Compare November 9, 2017 08:28
@mvz mvz self-assigned this Nov 9, 2017
Gemfile Outdated Show resolved Hide resolved
@stale
Copy link

stale bot commented Jan 28, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the stale These issues were closed by stalebot and need to be reviewed to see if they're still relevant. label Jan 28, 2018
@stale stale bot removed the stale These issues were closed by stalebot and need to be reviewed to see if they're still relevant. label Jan 28, 2018
@mvz mvz mentioned this pull request Feb 2, 2018
@olleolleolle
Copy link
Contributor

Can we rebase/merge master into this?

@mvz
Copy link
Contributor Author

mvz commented Mar 7, 2018

Yes, I'll do a rebase.

@mvz
Copy link
Contributor Author

mvz commented Mar 7, 2018

.. maybe even split it up so at least the build runs with the correct ruby versions.

@mvz mvz force-pushed the improve-appveyor-build branch 2 times, most recently from 099886e to da3a655 Compare March 11, 2018 20:49
@mvz
Copy link
Contributor Author

mvz commented Mar 11, 2018

Specs are green 🎉. Now for the features 🚧 👷‍♂️ 👷‍♀️ 🏗️

@olleolleolle
Copy link
Contributor

Congratulations on reaching a milestone on this! Getting specs to green is a summit to enjoy.

Next mountain will be possible to scale now that you’ve seen how high up you are.

(The only way is up.)

@olleolleolle
Copy link
Contributor

@mvz Was thinking of this branch, and how it makes the AppVeyor target list shorter - can THAT part be cherry-picked earlier than we fix all features for Windows support, too?

@mvz
Copy link
Contributor Author

mvz commented Mar 16, 2018

@olleolleolle I suppose all of it can be merged (once I fix the Travis build), since passing specs is better than failing specs.

@xtrasimplicity
Copy link
Member

@mvz Mind if I rebase and take a crack at this?

@mvz
Copy link
Contributor Author

mvz commented May 14, 2018

@xtrasimplicity please go ahead. As far as I remember, the current status is that specs are succeeding and cukes are failing. The main reason for the failing cukes is that Windows won't accept executables that have names without extensions, so all the scenarios that create some bin/foo script fail. The quick fix would be to just add a tag like @needs-unix to those scenarios.

@xtrasimplicity
Copy link
Member

Hey All,

I have some spare time this weekend to work on getting Aruba's test suite to work on Windows, and I was thinking I'd create Windows-specific versions of the current Linux-specific tests.

Given the current tests are used as documentation and I don't want to pollute the documentation with almost entirely-duplicated tests, I was thinking I might create a seperate folder under features/ for miscellaneous Windows-specific tests (that don't need to be included as documentation)?

Take the following scenario, for example. I don't think we need to have the exact same scenario duplicated beneath it, using echo %cd% or cd instead of pwd. Should I create a Windows-specific copy of this and save it under, say, features/09_windows_specific/cleanup_working_directory.feature? Or perhaps have a separate file under features/01_getting_started_with_aruba, called cleanup_working_directory_windows.feature?

# features\01_getting_started_with_aruba\cleanup_working_directory.feature
 Scenario: Clean up artifacts and pwd from a previous scenario
    Given a file named "features/cleanup.feature" with:
    """
    Feature: Check
      Scenario: Check #1
        Given a file named "file.txt" with "content"
        And a directory named "dir.d"
        Then a file named "file.txt" should exist
        And a directory named "dir.d" should exist
        When I cd to "dir.d"
        And I run `pwd`
        Then the output should match %r</tmp/aruba/dir.d$>
      Scenario: Check #2
        Then a file named "file.txt" should not exist
        And a directory named "dir.d" should not exist
        When I run `pwd`
        Then the output should match %r</tmp/aruba$>
    """
    When I run `cucumber`
    Then the features should all pass

For scenarios like the following, I would create a @requires-msdos or requires-cmd tag and create a MS-DOS/CMD prompt-specific feature, as it should be included in the documentation.

 @requires-bash
  Scenario: Bash Program run via bash
    Given a file named "features/hello_aruba.feature" with:
    """
    Feature: Getting Started With Aruba
      Scenario: First Run of Command
        Given a file named "cli.sh" with:
        \"\"\"
        echo "Hello, Aruba!"
        \"\"\"
        When I successfully run `bash ./cli.sh`
        Then the output should contain:
        \"\"\"
        Hello, Aruba!
        \"\"\"
    """
    When I successfully run `cucumber`
    Then the features should all pass

So, ultimately, how should I handle features that we don't want to appear in Documentation? I'd rather not omit them completely, as our test coverage on Windows will continue to suffer.

Any suggestions would be greatly appreciated. :)

Thanks!

@aslakhellesoy @olleolleolle @mvz -- Sorry for tagging all of you; just keen to get working on this as soon as I can. 👍

@mvz
Copy link
Contributor Author

mvz commented Jun 10, 2018

I see you found a solution :-).

@xtrasimplicity
Copy link
Member

Yep, I've added it to the 'unpublished' directory and updated the .cucumberproignore file. Happy to restructure this if need be - I'm just eager to get the AppVeyor build to pass. :)

@mvz
Copy link
Contributor Author

mvz commented Jun 10, 2018

I think this is the way to go. We can always move selected scenarios from unpublished into one of the published sections.

@@ -6,8 +6,10 @@
include_context 'uses aruba API'

describe '#run_command' do
let(:cmd) { 'ruby -ne "puts $_"' }
Copy link
Member

Choose a reason for hiding this comment

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

Windows doesn't have a command like cat that accepts input via stdin, prints it to stdout and accepts piped input, so this is the best I've come up with but it feels a little hack-y. @mvz, do you happen to know of any better ways to tackle this? Can you see any issues with this implementation? Thanks heaps!

Copy link
Contributor

Choose a reason for hiding this comment

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

Would something lets gets.chomp work here. Via an input on stdin? Genuinely not sure, so ignore me if not useful.

Copy link
Member

Choose a reason for hiding this comment

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

That almost works, but the process exits immediately afterwards. I can add a loop and trap an interrupt signal, but it's a fair bit more verbose. I've also found it doesn't quite function in the same way that cat does. Thanks heaps for the suggestion, though. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xtrasimplicity I can't remember what the Windows-equivalent command was. type, maybe? I kind of like your change because it's Just Ruby, but it may slow down tests on JRuby. Let's see how this goes for now.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, @mvz. type was almost perfect, from memory, but I don't think it handled SIGINT signals (or the windows equivalent) correctly. I'll keep it as is for the moment and see how it goes. :)


# Convert \r\n to \n, if present in the output
if last_command_output.include?("\r\n")
allow(@aruba.last_command_started).to receive(:output).and_return(last_command_output.gsub("\r\n", "\n"))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe better to either:

  • make have_output behave properly in this case,
  • provide a special verision of have_output that ignores changes to line endings.
  • or change the expectation below depending on whethere we're on Windows.

I think I prefer the third option for now.

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.

4 participants