-
-
Notifications
You must be signed in to change notification settings - Fork 906
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
Add script to run Linux tests in a container using Docker #767
Conversation
- [Reporting Bugs](#reporting-bugs) | ||
- [Building the Project](#building-the-project) | ||
- [Pull Requests](#pull-requests) | ||
- [Style Conventions](#style-conventions) | ||
- [Core Members](#core-members) | ||
- [Code of Conduct](#code-of-conduct) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this line is breaking some kind of rule from the QuickBot? That's weird because this change just came from running doctoc
like it says to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, this is from changing CONTRIBUTING.md
. I feel as though the documentation for this is most relevant in there, since it only impacts folks who want to contribute to Quick.
@modocache or @jeffh - What's the standard way to get around the block on making modifications to |
I don’t know of a standard way of getting around it. I have been busy with personal stuff for a while now and I haven’t had the time to be able to work on Quick stuff in a while. |
394c9ca
to
6f4514d
Compare
Refreshing this PR, as I continue to find it personally useful, and think others might as well. |
Generated by 🚫 Danger |
Dockerfile
Outdated
lsb-release \ | ||
rake | ||
|
||
RUN ./script/travis-install-linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the official Swift docker image is used, thers should be no need to do this I think. Swift is already there.
|
||
RUN ./script/travis-install-linux | ||
|
||
ENTRYPOINT ./script/travis-script-linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using swiftenv as said above, just running rake test:swiftpm
will be enough?
CONTRIBUTING.md
Outdated
- [Creating a Release](#creating-a-release) | ||
- [Testing for Linux from MacOS](#testing-for-linux-from-macos) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you place this next to the "Building the Project" section?
18a2f14
to
24b8715
Compare
@ikesyo - I have applied your suggestions above -- thank you for the review! As long as the tests here pass, I think this is good to merge in. |
@ikesyo - Seems like this check failed because it couldn't pull Nimble? It doesn't smell like it's related to my change, but I could be mistaken. |
@@ -1,4 +1,3 @@ | |||
#!/usr/bin/env sh | |||
|
|||
. ~/.swiftenv/init |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Travis jobs keep failing because of this change. Let's revert this as well and run rake test:swiftpm
directly in Dockerfile (as stated in https://github.com/Quick/Quick/pull/767/files#r249814869).
Closing as this is stale. |
I develop on a Mac. Yesterday, I broke the Linux build working on a PR to address the concerns raised by another issue, and found myself wishing I had an easy way to run the Linux test suite locally before pushing, lest I drive the community and myself crazy with the churn.
I ended up doing some stuff with Docker to see how easy that process could be, and that's where this PR comes from. It creates a new script that builds an image and uses it to run Quick's Linux tests. I smoke tested it with both a compilation error and an actual test failure and it works great!
This commit includes an update to the contributors' documentation explaining the test script and how to use it.
I understand that this is purely a quality-of-life change for the codebase's developers, and that we should be wary of those types of changes. I'd love feedback on this though, especially feedback on whether the community even believes this is a valuable addition to the repository.
Thanks, y'all 😃