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

RFC: Show build status for packages which have travis integration #3921

Merged
merged 1 commit into from
Aug 3, 2013

Conversation

aviks
Copy link
Member

@aviks aviks commented Aug 2, 2013

This PR displays the current build status for packages that have continuous integration running on Travis.

While this is a minority of packages currently, it will hopefully cause some social pressure on packages to add tests and to run them automatically.

See screenshot below on how it looks for packages with and without travis integration.

screen shot 2013-08-02 at 22 10 21

@timholy
Copy link
Sponsor Member

timholy commented Aug 2, 2013

Clearly this PR is a good idea. Your "social pressure" already worked on me for HDF5 (mostly simply to remind me to finish off what I started...)

I'll eventually add travis.yml files for some of my other projects.

@staticfloat
Copy link
Sponsor Member

+1

1 similar comment
@ViralBShah
Copy link
Member

+1

@timholy
Copy link
Sponsor Member

timholy commented Aug 2, 2013

I haven't followed the efforts to improve our testing as closely as I should, so I am probably about ten miles behind the pack here. But now that I think about it, isn't it perhaps a bit backwards to have a system in which:

  • Each package manager has to learn how to set up a Travis CI account and register their package(s)
  • Tests of a package are triggered by new commits to that package (as far as I can tell)

The problem with the second one is subtle, but probably more important: packages can also break due to changes in Julia, or changes in other packages. If I don't update my package, I won't discover the problem. (It also misses an important opportunity, that packages represent additional tests on Julia itself.)

Wouldn't it be better to have each package tested nightly along with Julia? I wonder if we could set it up this general way:

  • The overall Travis build fails only if Julia fails its own internal tests
  • To ensure this, each tested package has a wrapper around it that will lie to Travis and indicate success, no matter what happens
  • In the "catch" block of this wrapper, we write
    sendmail(pkgauthor, "Your package $pkg failed its tests on $date")
  • At the end, a file is uploaded somewhere giving a summary of the true status of each package.

I think we lack some of these pieces, but presumably something along these lines wouldn't be that big of a deal to implement?

@staticfloat
Copy link
Sponsor Member

Tim, you're on the same wavelength as Viral, Iain and I. ;)

Once we finish Julep 2, I plan on doing exactly what you suggest; running
nightly tests of Julia against all the packages in METADATA. This will give
us an idea of when Julia "breaks" a package.
On Aug 2, 2013 12:51 PM, "Tim Holy" notifications@github.com wrote:

I haven't followed the efforts to improve our testing as closely as I
should, so I am probably about ten miles behind the pack here. But now that
I think about it, isn't it perhaps a bit backwards to have a system in
which:

  • Each package manager has to learn how to set up a Travis CI account
    and register their package(s)
  • Tests of a package are triggered by new commits to that package (as
    far as I can tell)

The problem with the second one is subtle, but probably more important:
packages can also break due to changes in Julia, or changes in other
packages. If I don't update my package, I won't discover the problem. (It
also misses an important opportunity, that packages represent additional
tests on Julia itself.)

Wouldn't it be better to have each package tested nightly along with
Julia? I wonder if we could set it up this general way:

  • The overall Travis build fails only if Julia fails its own internal
    tests
  • To ensure this, each tested package has a wrapper around is that
    will lie to Travis and indicate success, no matter what happens
  • In the "catch" block of this wrapper, we write sendmail(pkgauthor,
    "Your package $pkg failed its tests on $date")
  • At the end, a file is uploaded somewhere giving a summary of the
    true status of each package.

I think we lack some of these pieces, but presumably something along these
lines wouldn't be that big of a deal to implement?


Reply to this email directly or view it on GitHubhttps://github.com//pull/3921#issuecomment-22030896
.

@timholy
Copy link
Sponsor Member

timholy commented Aug 2, 2013

See, you are about 10 miles ahead of me, even if I'm tuning in to the same radio station :-).

@IainNZ
Copy link
Member

IainNZ commented Aug 2, 2013

Link for reference to Julep 2: https://gist.github.com/IainNZ/6086173
I think this is a good pull request to get us along this path though.
Also see my PackageEvaluator code that I'm going to be fleshing out this weekend, that will hopefully be pulled into... somthing: https://github.com/IainNZ/PackageEvaluator.jl

@mlubin
Copy link
Member

mlubin commented Aug 2, 2013

See my comment in the gist. I'm in favor of some form of this, but I think it should be smoothed out. The point is to give users a sense of the quality of packages and shame package maintainers, but there's a lot of noise in travis build failures. This PR is good to get things started, but developing a more comprehensive test infrastructure with julep 2 is a more long term solution.

@IainNZ
Copy link
Member

IainNZ commented Aug 2, 2013

@mlubin Agreed.
We've had Travis failures for IainNZ/JuMP.jl - in fact, the night I first set it up, it failed because the nightly was broken. I iike the progress this PR represents, but should it be merged in as-is?

@mlubin
Copy link
Member

mlubin commented Aug 2, 2013

It probably does more good than harm by encouraging package maintainers to set up travis, so it's ok with me to merge.

@aviks
Copy link
Member Author

aviks commented Aug 3, 2013

Yes, running a nightly test against all packages is a good idea which I know you guys are working on, but it wont be of much value unless most packages have automated tests. My idea is that having this display will motivate maintainers to do so. So I'll merge it for now. Can be reverted easily when the social pressure is no longer necessary.

Also, I suppose that currently, given the fast moving nature of Julia, it is incumbent on package authors to update packages when Julia changes.... there are no backward compatible guarantees, for good reason. (the package tests are always run with Julia nightly). Of course, as @timholy says, it is a problem that the travis tests are run for package commits. So running a smoke test against all packages is certainly valuable.

The simplest way of enabling travis currently is to copy and make minor modifications to the file in the Example.jl repository. With Julep2 and Pkg2, I believe we can reach a state where no modification is necessary on that file.

aviks added a commit that referenced this pull request Aug 3, 2013
RFC: Show  build status for packages which have travis integration
@aviks aviks merged commit 02c120a into JuliaLang:master Aug 3, 2013
@aviks aviks deleted the pkg-travis branch August 3, 2013 06:47
@timholy
Copy link
Sponsor Member

timholy commented Aug 3, 2013

It probably does more good than harm by encouraging package maintainers to set up travis, so it's ok with me to merge.

I have no objections to this being merged, but I do think we shouldn't be coming at this with the expectation that our package managers will go through this. I found setting up Travis surprisingly unpolished and non-intuitive:

  • When I go to https://travis-ci.org/, "My repositories" shows things already set up on Travis, mostly by other people (because I am on JuliaLang). I can't manipulate the settings for what happens to those packages, or add new ones, from that page. To set up testing on a new repository, it's not obvious that I have to click on my icon at the top right and select "accounts". (What does "accounts" have to do with my GitHub repositories? "accounts" is where I expect to change my Travis password, etc.)
  • Turning the slider "on" for one of my repositories is the easy part. Once you hit the wrench, it's surprising that the first step you have to do is select "Travis" (I was just at Travis...). There's no message indicating that you've been redirected to GitHub.
  • Once you select Travis, the "Install Notes" are quite incomplete: finding your token is poorly described (again it requires hunting around to find on a separate page whose name did not obviously pop out from the words "profile page"), the "Test Hook" button appears on the GitHub page after I, or it, had navigated back to Travis (I was expecting to find it on the Travis website, not the GitHub website).
  • Nothing anywhere says that to trigger a build you need to push a commit. If I hadn't noticed @Avik's message about this trick (which was apparently discovered by trial and error), I wouldn't have known this.

Overall I think this process is too bumpy. We already have one barrier to entry, git (easy for programmers, but "98% of all biomedical scientists" stare at you blankly when you ask), but at least GitHub provides a lovely interface for basic management of repositories. This one cannot be described as lovely. Either Travis & GitHub need to improve this process and/or their docs, or we need to find a way of handling this for people.

@ViralBShah
Copy link
Member

Why not capture this all in the packaging section of the manual?

@ViralBShah
Copy link
Member

Also this is a nudge rather than something mandatory.

@nolta
Copy link
Member

nolta commented Aug 3, 2013

I think this is a bad idea. The status of master is irrelevant. Only the shas in METADATA matter.

Are we seriously going to mark packages as bad, even if the versions checked out by Pkg.add work fine?

@mlubin
Copy link
Member

mlubin commented Aug 3, 2013

@nolta, that's a good point. Maybe the best thing to do now before nightly testing infrastructure is set up is to just indicate whether or not a package has travis set up? (without providing the build status)

@IainNZ
Copy link
Member

IainNZ commented Aug 5, 2013

@nolta is very right. The more I think through the implications of @nolta 's point, the more complicated things get.
I've put my thoughts here: https://groups.google.com/forum/#!topic/julia-dev/DDRAncCKrm0 , hope they provoke some good ideas.

@aviks
Copy link
Member Author

aviks commented Aug 22, 2013

@nolta My thinking was that a broken master signals an unmaintained package even if the released version is clear. But you do have a point, maybe its not a very strong signal.

OK, so given that people don't think this is a good idea any more, I am tempted to revert this change. Would it be better to include a link to the Travis page, rather than the badge? Or just revert to previous state?

@mlubin
Copy link
Member

mlubin commented Aug 22, 2013

I'd vote for just indicating whether or not the project has travis integration. That's the most useful information we can provide before the testing infrastructure is set up.

@aviks
Copy link
Member Author

aviks commented Sep 7, 2013

We now just show a Travis icon with a link to the project's travis page for packages that have Travis integration. The build status badge has been removed. Via 54405d9

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.

None yet

7 participants