Code

Opened 14 months ago

Last modified 4 months ago

#19891 assigned Cleanup/optimization

Travis CI support

Reported by: marko@… Owned by: gcc
Component: Testing framework Version: master
Severity: Normal Keywords: Travis, CI, testing
Cc: mathijs@…, apollo13 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 aaugustin 14 months ago.

Download all attachments as: .zip

Change History (34)

comment:1 Changed 14 months ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 14 months ago by dokterbob

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 14 months ago by dokterbob (previous) (diff)

comment:3 Changed 14 months ago by jacob

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 Changed 14 months ago by jacob

  • Triage Stage changed from Unreviewed to Accepted

comment:5 Changed 14 months ago by dokterbob

  • Cc mathijs@… added
  • Has patch set
  • Owner changed from nobody to dokterbob
  • Status changed from new to assigned

comment:6 Changed 14 months ago by dokterbob

  • Easy pickings set

Any news on this one?

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

Any further modifications needed?

comment:8 Changed 14 months ago by apollo13

  • Cc apollo13 added

comment:9 Changed 14 months ago by aaugustin

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 Changed 14 months ago by dokterbob

@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.

Changed 14 months ago by aaugustin

comment:11 Changed 14 months ago by aaugustin

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

comment:12 Changed 14 months ago by dokterbob

@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 Changed 14 months ago by apollo13

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 Changed 14 months ago by dokterbob

@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 Changed 13 months ago by apollo13

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 Changed 13 months ago by camilonova

+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 Changed 13 months ago by apollo13

@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 Changed 11 months ago by dokterbob

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 Changed 11 months ago by anonymous

  • Owner dokterbob deleted
  • Status changed from assigned to new

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

comment:21 Changed 11 months ago by gcc

@dokterbob I think from the discussion above that we don't want Travis to build master or any other branch of django, only pull requests? Core devs have their own Jenkins setups to run tests for them.

So polluting the timeline is not a problem, because it should never be necessary to make a commit on any branch of django/django to trigger a Travis build because these branches are not built by Travis. 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.

Version 0, edited 11 months ago by gcc (next)

comment:22 Changed 11 months ago by gcc

  • Owner set to gcc
  • Status changed from new to assigned

comment:23 Changed 11 months ago by gcc

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 Changed 11 months ago by gcc

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 Changed 11 months ago by apollo13

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 follow-up: Changed 11 months ago by gcc

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.

comment:27 in reply to: ↑ 26 Changed 11 months ago by apollo13

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 Changed 10 months ago by gcc

  • 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 Changed 8 months ago by timo

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 Changed 8 months ago by timo

  • Triage Stage changed from Accepted to Someday/Maybe

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

comment:31 Changed 8 months ago by dokterbob

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 Changed 5 months ago by bouke

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 5 months ago by bouke (previous) (diff)

comment:33 Changed 4 months ago by mjtamlyn

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 Changed 4 months ago by gcc

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as assigned
The owner will be changed from gcc to anonymous. Next status will be 'assigned'
The ticket will be disowned. Next status will be 'new'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.