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

Add necessary integration pieces for Travis CI #19

Merged
merged 1 commit into from
Apr 4, 2017

Conversation

ngie-eign
Copy link
Collaborator

@ngie-eign ngie-eign commented Apr 3, 2017

Add necessary integration pieces for Travis CI

  • .travis.yml: provides the metadata for describing how the project should be built and tested.
  • travis/build.sh: builds the project.
  • travis/test.sh: tests the project.

This commit adds support for the following matrix:

  • Ubuntu 16.04 LTS (xenial) x {clang,gcc} x {ext4}

Support for OS X will be added once runtime issues with it have been resolved.

Some administrative switches need to be flipped in order for this change to be made effective, but this has proven successful on my fork (yaneurabeya/pjdfstest).

Capture client information for FreeBSD, Linux, and OS X

FreeBSD isn't supported with Travis CI, but Linux definitely is, and OS X is partially supported.

Fixes GitHub issue #12.

@ngie-eign ngie-eign self-assigned this Apr 3, 2017
@ngie-eign ngie-eign requested a review from asomers April 3, 2017 18:24
Copy link
Collaborator

@asomers asomers left a comment

Choose a reason for hiding this comment

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

How do we specify what Docker image to use? Do you know what filesystem Travis uses?

travis/test.sh Outdated
set -e

cd $(dirname $0)/..
sudo prove -rv .
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will create the temporary files in the source directory, which isn't what we want if the source directory is a network filesystem. Should we use /tmp instead?

@asomers
Copy link
Collaborator

asomers commented Apr 4, 2017

IMHO we shouldn't bother testing on Trusty. We should only bother with the latest version of each supported OS, since the purpose of pjdfstest is to catch FS bugs as they're being developed.

@ngie-eign ngie-eign force-pushed the travis-integration branch from ce65ea7 to 6549aef Compare April 4, 2017 04:30
@ngie-eign
Copy link
Collaborator Author

@asomers: I disagree. We aren't really losing much by running on trusty, and if I'm able to catch bugs on 14.04 LTS, it means that I don't need to fire up a VM/make sure that everything always works on that version (unless I need to, in which case I will indeed install it/start it up).

I think (latest stable) - 1 releases is an ok support model. It would be the equivalent of running/supporting pjdfstest on 10.x and 11.x.

@ngie-eign
Copy link
Collaborator Author

@asomers: alternatively, we could run trusty at a lower cadence, test wise. I'm not incredibly keen on doing this, but it's a potential option.

@asomers
Copy link
Collaborator

asomers commented Apr 4, 2017

@ngie that depends on why we're running CI on Trusty. Are we doing it to ensure that pjdfstest doesn't break, or to ensure that Trusty doesn't break? If the former, then all we really need to do is make sure that pjdfstest.c compiles. The .sh syntax is so portable that I doubt we'll ever find a syntax bug on one OS but not another. If the latter, then we really need to be running this in Canonical's infrastructure, not pjdfstest's.

@ngie-eign
Copy link
Collaborator Author

ngie-eign commented Apr 4, 2017

I was caring more about the latter case right now, in particular because I'm going to be adding autoconf support to pjdfstest and in doing so I might run into development issues either because of tool versioning or because syscalls/libcalls previously not lit up, e.g. posix_fallocate, might not function the same way.

I'm more than happy to scale back the runs once things stabilize, or move to Canonical's infrastructure. Using Travis CI's infrastructure was the lowest hanging fruit for me.

@asomers
Copy link
Collaborator

asomers commented Apr 4, 2017

Are you sure you mean the latter case? You're trying to regression test ubuntu 14.04?

@ngie-eign
Copy link
Collaborator Author

ngie-eign commented Apr 4, 2017

For right now, yeah. That way I don't have to go back and fix something N months down the line with a particular version of Linux...

@asomers
Copy link
Collaborator

asomers commented Apr 4, 2017

So you're developing filesystems for old versions of Linux?

@ngie-eign
Copy link
Collaborator Author

ngie-eign commented Apr 4, 2017

No, my employer doesn't develop products based on Linux (we consume it as a client platform).

Ubuntu (for better/worse) is a defacto standard Linux distro that many in the community use to develop on (including some people on my team/at my work). I would rather catch weirdness in clients (if possible), so we can push Ubuntu to fix their issues instead of pushing hacks into our product :).

Granted, this should probably also be tested using NFS, not ext4, but if ext4 doesn't function, the likelihood of NFS functioning is probably lower.

@ngie-eign ngie-eign force-pushed the travis-integration branch from 309a8cb to 26e7ec9 Compare April 4, 2017 19:14
@asomers
Copy link
Collaborator

asomers commented Apr 4, 2017

pjdfstest's TravisCI instance is simply the wrong place to look for bugs in each and every Linux distro, especially old ones. By adding Trusty to the test matrix, you're saying that if Canonical pushes out a bug in a Trusty update, then all development of pjdfstests should halt until that bug is fixed. Ideally pjdfstest would only be tested on some magical golden OS that was guaranteed to have no filesystem bugs. Such a thing doesn't exist, so using one recent version of Linux is a decent compromise. But we shouldn't turn pjdfstest's Travis instance into the world's filesystem testbed.

- .travis.yml: provides the metadata for describing how
  the project should be built and tested.
- travis/build.sh: builds the project.
- travis/test.sh: tests the project.

This commit adds support for the following matrix:
- Ubuntu 16.04 LTS (xenial) x {clang,gcc} x {ext4}

Support for OS X will be added once runtime issues with it
have been resolved.

Some administrative switches need to be flipped in order for
this change to be made effective, but this has proven
successful on my fork (yaneurabeya/pjdfstest).

Capture client information for FreeBSD, Linux, and OS X

FreeBSD isn't supported with Travis CI, but Linux definitely
is, and OS X is partially supported.

Fixes GitHub issue pjd#12.
@ngie-eign ngie-eign force-pushed the travis-integration branch from 26e7ec9 to f8d02e3 Compare April 4, 2017 19:41
@ngie-eign
Copy link
Collaborator Author

Ok, I'll cave.

@ngie-eign
Copy link
Collaborator Author

Is there anything else you would like me to address in this review?

@asomers
Copy link
Collaborator

asomers commented Apr 4, 2017

Nope, otherwise it all looks good to me.

@asomers asomers merged commit 93004fa into pjd:master Apr 4, 2017
@ngie-eign ngie-eign deleted the travis-integration branch April 4, 2017 20:01
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