Opened 11 years ago

Closed 10 years ago

Last modified 8 years ago

#19891 closed Cleanup/optimization (wontfix)

Travis CI support

Reported by: marko@… Owned by: Chris Wilson
Component: Testing framework Version: dev
Severity: Normal Keywords: Travis, CI, testing
Cc: mathijs@…, Florian Apolloner Triage Stage: Someday/Maybe
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Use cases

Standardizing testing

Contributors test their own changes against all different databases and supported python versions and testing is standardized.

This means they do not have to setup configure and maintain multiple database setups, install multiple python versions etc.

Pull requests get tested automatically

This gives core-devs a first impression on the quality of a patch.

History

@apollo13 has made some initial attempts at supporting Travis CI, but stopped working on this
because the tests were taking too long and were causing timeouts. New work being done is based
on his initial attempts.

Development

Work on this topic is currently being done at https://github.com/dokterbob/django

Discussion

Not everyone is too fond of travis-ci. Sometimes people tend to pollute the commit history with commits just to
trigger Travis CI.

The work being done is not to replace Jenkins CI. It is only intended to improve and standardize testing of Django by
none-core developers.

Attachments (1)

travis.eml (8.1 KB ) - added by Aymeric Augustin 11 years ago.

Download all attachments as: .zip

Change History (36)

comment:1 by Jacob, 11 years ago

I don't think having Travis test all versions is such a good idea. Right now the build generates 12 different builds for each pull request which'll take approximately forever to run. Asking people to wait on pull requests while a dozen different builds run is... not so good.

Instead, how about using Travis to augment what we already have in Jenkins? Just have it run maybe two builds -- Python 2 and Python 3? That was contributors can get a quick feedback on a pull request, but don't have to wait forever.

comment:2 by Mathijs de Bruin, 11 years ago

There's a pull request now, running the tests:
https://github.com/django/django/pull/805

The tests take 25-45 minutes to run, which is not 'forever' AFAIK. Travis runs 3-5 builds in parallel which actually makes it fairly fast. However, somehow, Travis adds up the time for the individual builds to come up with a rather unrealistic build time of multiple hours.

As the core dev's have a Jenkins CI anyways, this shouldn't matter too much for them. However, many contributors would benefit tremendously by being able to test their code throughly even before firing pull requests - which is the main point of this patch. And lastly, it *might* happen that Travis finds issues which (somehow) slip through the mazes of the Jenkins CI, as was the case while we set up the testing system. (Ref: https://github.com/django/django/commit/8c1cc4b3b04f639ae40bc806380bc4a9dd568eb0)

However, having it run on 4 Python versions might indeed be too much. Which two Python releases do you suggest we use?

Lastly: as the core team already has their Jenkins CI setup we recommend *not* switching on Travis for the main Django repo right now. This, however does not prevent contributors from doing so and providing a .travis.yml file will make setting up standardized testing a breeze. And if tests do pass, the information will be provided as clear feedback in the pull requests - as a bonus (not a necessity).

Last edited 11 years ago by Mathijs de Bruin (previous) (diff)

comment:3 by Jacob, 11 years ago

Hm, 45 minutes isn't bad; Travis (or our tests) must have gotten faster since last I checked. So I'll drop my objection.

comment:4 by Jacob, 11 years ago

Triage Stage: UnreviewedAccepted

comment:5 by Mathijs de Bruin, 11 years ago

Cc: mathijs@… added
Has patch: set
Owner: changed from nobody to Mathijs de Bruin
Status: newassigned

comment:6 by Mathijs de Bruin, 11 years ago

Easy pickings: set

Any news on this one?

Will it me merged? If not; what's blocking it?

Any further modifications needed?

comment:8 by Florian Apolloner, 11 years ago

Cc: Florian Apolloner added

comment:9 by Aymeric Augustin, 11 years ago

I simply hope that I won't receive too many emails indicating failures of Travis' infrastructure.

During the sprint I received an email informing me that I had broken the build. The email pointed to a docs commit. No comment!

comment:10 by Mathijs de Bruin, 11 years ago

@aaugustin Could you forward the mail to me? I'm *very* curious as to why you'd receive mail for tests in a repo which isn't yours. In any case such emails should be disabled until Travis' stability is sufficiently proven - this should mainly be a tool for use by contributors *before* the code gets to you guys.

by Aymeric Augustin, 11 years ago

Attachment: travis.eml added

comment:11 by Aymeric Augustin, 11 years ago

I just attached the email (I scrubbed the headers first).

comment:12 by Mathijs de Bruin, 11 years ago

@aaugustin Actually, the changeset seems to contain a whole bunch of changes. I remember that during the sprint for a while the master branch was broken which might have caused the e-mail.

But really, we don't want to bother you guys with tons of emails for broken/fixed pull requests. Hence, I propose adding to the .travis.yml:

notifications:
  email: false

However, having notifications on IRC might be nice. I'm not an avid user myself so I'm open to any suggestions. Refer to: http://about.travis-ci.org/docs/user/notifications/#IRC-notification

comment:13 by Florian Apolloner, 11 years ago

However, having notifications on IRC might be nice. I'm not an avid user myself so I'm open to any suggestions. Refer to: ​http://about.travis-ci.org/docs/user/notifications/#IRC-notification

Absolutely not, we don't want notifications for pull requests etc.

Also the email issue has to be figured out, travis is an absolute nogo if people start receiving mails while Django doesn't even have a travis.yml file (Independent of what the notification setting is).

comment:14 by Mathijs de Bruin, 11 years ago

@apollo13 I was surprised about that as well. Perhaps you could ask around on #travis what's happening here?

As the default behaviour of Travis is to be verbose, I suppose switching email notifications off will do the trick for now.

comment:15 by Florian Apolloner, 11 years ago

Ugh, I retried my separate config approach and I am getting crazy errors all along: https://api.travis-ci.org/jobs/5486568/log.txt?deansi=true (https://travis-ci.org/apollo13/django/builds/5486564). Also fwiw: loading the last job with a failed log completely freezes Firefox. I am once again wondering how it worked for you :þ

comment:16 by Camilo Nova, 11 years ago

+1 on this, will be great to have it, seems more easy to read the config file from dokterbob and adding the configuration for not sending emails should work fine as well http://about.travis-ci.org/docs/user/notifications/#Email-notifications

What else is needed to have this merged? i'm glad to help if needed

comment:17 by Florian Apolloner, 11 years ago

@camilonova: As discussed on the mailing list we are going with the "harder" approach, that is fetching travis config from a 3rd server and keeping the travis file as small as possible…

Regarding help, well forking my fork ;) and figuring out why it's broken would help, I am currently out of ideas.

comment:19 by Mathijs de Bruin, 11 years ago

TLDR; Single commit everytime Travis tests break should be no problem, but if it is will work on Travis repo.

@apollo13 The talk just now at DjangoCon made me remember this issue and read the related thread on the mailinglist. Have had no time to actually discuss on the dev' list but will present some arguments here.

Personally I don't think it's elegant having a seperate repository just to keep a couple of commits out of the public timeline. Most Travis issues can be easily fixed in a fork/pull request as the exact same tests are run under the exact same circumstances there - part of the point of having Travis there in the first place.

Having a seperate repo with just a config file potentially makes the tests more fragile as a change to the Travis config could cause tests to fail for no clearly apparent reason.

If I could talk you guys into having Travis support in the main Django repo I will to the following:

  1. Pick up old work; rebase my branch to current master.
  2. Disable e-mail and IRC notifications for core dev's piece of mind.
  3. Run the tests again a couple of times to see if anything breaks.
  4. Create a single cute litte patch so there'll no 'fixing Travis' in the public timeline for now.

Anyways, if the core devs do decide to go with a split-repo approach to Travis I'm willing to work on it during DjangoCon EU and will either finish it or leave it 'open' by the beginning of the sprints (will take train on friday). What I need to do this is:

  1. (Temporary) access (directly or indirectly, through pull req's) to a travis-support GH repo.
  2. A quick overview of what happened on this since the latest sprint from apollo13.
  3. Clear instructions on what the new approach will entail, what the goals are and what the conditions are under which it will be committed.

Having Travis for Django still seems like a really good idea which could significantly increase the (quality of) contributions. In any case, I'm walking around on DjangoCon until friday afternoon. If you have questions or want to work on me with this one, poke me. If you are a core dev and want to discuss, poke me. If you're not a core dev, poke a core dev to get this in (preferably before the sprints). Then, during the sprints, we'll see quick enough whether Travis will prove to be a curse or a blessing but I'm counting on the latter to be the case.

comment:20 by anonymous, 11 years ago

Owner: Mathijs de Bruin removed
Status: assignednew

I'm leaving this for others to work on during the DjangoCon EU sprints as it is not going to happen today.

comment:21 by Chris Wilson, 11 years ago

@dokterbob I think that:

  • Core devs have their own Jenkins setups to run tests for them.
  • So we don't care that Travis builds any branch of Django, only about the building of pull requests.
  • It appears that we can't disable building branches, but we don't need to do anything if a branch build fails, because we don't care.
  • So we should never need to make a commit on a branch just to force a Travis build.

Is that correct?

I had trouble getting started on closing this ticket so I've made a TODO list of the outstanding issues. I hope this is useful.

Last edited 11 years ago by Chris Wilson (previous) (diff)

comment:22 by Chris Wilson, 11 years ago

Owner: set to Chris Wilson
Status: newassigned

comment:23 by Chris Wilson, 11 years ago

Postgres on Travis crashes when we run all the tests due to ​running out of disk space. It seems that this might happen when ​disk writes are slow, which I can imagine happening in a virtual machine. Can any Postgres expert help debug and fix this?

The current state of my work is in github/aptivate/django.

comment:24 by Chris Wilson, 11 years ago

Pull request.

This branch adds the necessary support for Travis into Django, as discussed on [ticket #19891](https://code.djangoproject.com/ticket/19891).

It runs all Django tests on the following environments:

  • SQLite, no GIS
  • MySQL, with and without GIS
  • PostgreSQL, with and without GIS

It also builds the docs. It does not currently run Selenium tests (because it cannot run ChromeDriver, even though it's executable and in the PATH).

Due to some quirks in Travis, it will try to build branches even if they have no .travis.yml file. It will try to build them as Ruby code, which will fail. Therefore this file must be created with some minimal contents in all regularly-changing branches of Django before enabling Travis, otherwise people will get email notifications when those branches are modified, triggering a Travis build which will fail. I suggest using a copy of .travis.yml from this branch.

I have also seen the build time out when running PostgreSQL tests (over 50 minutes). This may well happen again. We might need to split the PostgreSQL tests into two separate runs.

Many thanks to @dokterbob and @apollo13 who did all the hard work that made this possible.

comment:25 by Florian Apolloner, 11 years ago

Hi, there are a few issues with the PR:

  • As I said before I don't wan't the config stuff to be in the repo
  • The postgres fixes are crazy, please get in touch with the travis guys (I don't think writes are slow they moved to ramfs to my knowledge)
  • "emails: false" is not enough, this doesn't prevent someone from re enabling it in his PR and then spamming everyone else…
  • Splitting PG tests is not an option.
  • Selenium used to work, why can't it run chromedriver all of a sudden?


comment:26 by Chris Wilson, 11 years ago

Hi @apollo13,

Please could you tell me why you don't want config stuff in the repo? Loading them from an outside web server:

  • is dangerous (security risk to Travis and hence our reputation),
  • is unreliable (might go away suddenly with no warning),
  • makes it impossible to test new configs if you don't have write access to the repo where they're stored, and
  • eliminates them from version control, or at least separates them into a different repo or VCS for no reason that I can see.

I have already been in touch with Travis guys about the Postgres issues as I could not find any other way to stop the tests failing due to running out of disk space. What's crazy about them?

Will look into the email issue, but I'm not sure we can do anything except creating a new account with a /dev/null email address and making it a collaborator on the django repo. Will look into Selenium as well.

Why is splitting the PG tests not an option?

Cheers, Chris.

in reply to:  26 comment:27 by Florian Apolloner, 11 years ago

Replying to gcc:

Please could you tell me why you don't want config stuff in the repo?

As I already said on the mailinglist: I don't want to have commits in Django's timeline saying: "And another fix for travis, oh and another one for travis again". It's bad enough that they require me to add a .travis.yml, CI shouldn't be bound to a repo like this.

  • is dangerous (security risk to Travis and hence our reputation),

Why would that be a security risk?

  • is unreliable (might go away suddenly with no warning),

More unreliable than Travis? Also once we deploy this we will either fetch from dp.com or similar, so it surely won't just go away.

  • makes it impossible to test new configs if you don't have write access to the repo where they're stored, and

No, you just have to change the environment variable in your branch and point to your server.

  • eliminates them from version control, or at least separates them into a different repo or VCS for no reason that I can see.

The main idea for this split was to be able to version them in a separate repository; after all CI-config depends on the one running the CI and shouldn't be in the main repo.

What's crazy about them?

Cause it's completely fragile, if travis changes from /var/ramfs to something else your builds start failing.

Will look into the email issue, but I'm not sure we can do anything except creating a new account with a /dev/null email address and making it a collaborator on the django repo.

I doubt that will help, Travis (at least that's how I understood it) looks at the email addy of the commits that got pushed to the repo and then bugs people accordingly.

Why is splitting the PG tests not an option?

Cause it introduces more complexity, this should be a last resort.

comment:28 by Chris Wilson, 11 years ago

Easy pickings: unset

Hi @apollo13,

Status update:

  • I also moved the Travis configuration scripts and files to a Git repo so that I don't have to host them somewhere (except in Github) to run tests in Travis. I hope that's an acceptable alternative to HTTP hosting.
  • I added support for UNLOGGED tables that should speed up running tests in Postgres for all users. That could be considered a separate branch/ticket if necessary.
  • Selenium tests are still broken. I want to see if I can overcome the other issues first before investing time in fixing them.

You can see the latest work here. Please let me know if these things are acceptable. If so then I'll keep working on fixing selenium tests.

comment:29 by Tim Graham, 11 years ago

Talking to apollo13 on IRC, it seems like the email notification issue is indeed a blocker to merging this. We discussed the possibility of adding pull request support to our Jenkins setup. We need to add additional executors to Jenkins and make it more stable before we can proceed on that.

comment:30 by Tim Graham, 11 years ago

Triage Stage: AcceptedSomeday/Maybe

If Travis fixes email notifications, we can move this back to "Accepted" and continue work.

comment:31 by Mathijs de Bruin, 11 years ago

There is an issue for Travis suggesting to move notification settings away from the .travis.yml file and into the database. This would probably fix the notification problems for forks/pull requests.

https://github.com/travis-ci/travis-ci/issues/1094

comment:32 by Bouke Haarsma, 10 years ago

There's a policy change landing next week (27 nov) worth looking into:

The new policy will be as follows:

  1. If the commit occurs on the default branch, then the owners of the repository will be notified.
  2. If the commit occurs on a non-default branch, the author and the committer of the commit who are also owners of the repository will be notified.

http://about.travis-ci.org/blog/2013-11-19-upcoming-email-notification-policy-change/

Also django-cms/.travis.yml has a complete travis setup, including sqlite, postgres and mysql. Could be of inspiration for Django's travis config.

Last edited 10 years ago by Bouke Haarsma (previous) (diff)

comment:33 by Marc Tamlyn, 10 years ago

My instinct is that this change, combined with a notifications: false in the config should be sufficient to prevent spam.

Apollo's other comments still stand though, and in particular if the selenium tests continue to fail then we have a blocking issue.

comment:34 by Chris Wilson, 10 years ago

Also please confirm that the other changes I made in my branch are acceptable. If so then I'll look at fixing Selenium. Otherwise I give up.

comment:35 by Tim Graham, 10 years ago

Resolution: wontfix
Status: assignedclosed

We now have a scalable Jenkins setup with pull request integration at djangoci.com.

comment:36 by Tobias McNulty, 8 years ago

I created a "lightweight" Travis/tox setup for my own purposes so I can check pull requests before submitting them to Django proper. The goal is not to replace traviceci.com but to provide a lightweight sanity check and help avoid clogging up Travis when making frequent commits.

Anyone who stumbles across this ticket is welcome to copy what I did to your own GitHub/Travis account or submit a PR to my fork to see if it builds. I will not, of course, review your PR or merge it into this fork. There are no guarantees that it'll be completely up to date (create a fork for yourself if needed).

The repo is here, on the "travis" branch: https://github.com/tobiasmcnulty/django

Note: See TracTickets for help on using tickets.
Back to Top