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

Migrate integration tests to shell based format #2864

Closed
wants to merge 2 commits into from

Conversation

BardurArantsson
Copy link
Collaborator

This is an implementation of #2797.

Let me know if further description/explanation is necessary and I'll add it to the commit message(s).

This is required for the reworking of integration tests
to use shell scripts.
@BardurArantsson
Copy link
Collaborator Author

@grayjay Could you please try this out on Windows and let me know what is still missing relative to fully working on Windows?

(I think I fixed all the test cases + line ending things, but I could easily have missed something.)

@BardurArantsson
Copy link
Collaborator Author

Hm... that's an interesting failure. What I'm doing is copying a two-level directory hierarchy from the source directory into a temporary folder and running a shell script in the 2nd level directory. Apparently "source ../foo.sh" isn't picking up the "foo.sh" that should have been copied into the 1st level directory.

Anyone have any ideas what's going on with Travis here?

(I would have guessed a CWD issue could have caused this, but I explicitly set the CWD when running the shell script that does the "source" thing.)

@BardurArantsson
Copy link
Collaborator Author

Oh, wait, maybe I'm setting CWD using a relative path. I'll check that tomorrow.


# When run inside 'cabal-exec' the 'sandbox hc-pkg list' sub-command
# should find the library.
cabal exec sh -- -c "cd subdir && $CABAL sandbox hc-pkg list" | grep "my-0.1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

CABAL contains backslashes on Windows, so I had to quote it here. The quoted path still doesn't work with Cygwin, though.

@grayjay
Copy link
Collaborator

grayjay commented Oct 11, 2015

@BardurArantsson This branch works well on Windows, except for three things:

  • The path to sh is hard-coded as /bin/sh.
  • The variables CABAL and GHC_PKG refer to Windows-style paths. I tested with sh from MinGW and Cygwin this time and found that only MinGW can handle the paths.
  • (See the comment in the code.)

@BardurArantsson
Copy link
Collaborator Author

Thanks.

I'm not sure what we can do about "/bin/sh": That's actually close to being the only guaranteed way to get a shell on a POSIX(ish) system. The only other one I know of is to use "env". Does "/usr/bin/env sh" work for you?

(I suppose we could do a lookup for /bin/sh first and try plain "sh" if "/bin/sh" isn't found.)

I don't think supporting Cygwin is necessarily a goal since (I believe) msys is the "primary" Windows-compatibility system... @ezyang: Do you concur?

Will fix the quoting issue.

@grayjay
Copy link
Collaborator

grayjay commented Oct 12, 2015

@BardurArantsson "/usr/bin/env sh" doesn't work because the path is different on Windows ("C:/Program Files/Git/usr/bin/env" on my computer). Trying "/bin/sh" and then "sh" sounds like a good solution.

@23Skidoo
Copy link
Member

Yes, lack of Cygwin support is not a blocker. Only supporting msys2 is fine. But we must get this to work on Travis before merging.

@23Skidoo
Copy link
Member

Closing in favour of #2868, which subsumes this PR.

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

Successfully merging this pull request may close these issues.

3 participants