search

metomi / isodatetime

:date: :watch: Python ISO 8601 date time parser and data model/manipulation utilities
GNU Lesser General Public License v3.0
37 stars 20 forks source link

travis: run tests in 4 processes #164

Closed oliver-sanders closed 4 years ago

oliver-sanders commented 4 years ago

Might a well give this a go whilst we are here.

This change allows you to run the really slow test using as many processes as you want.

MetRonnie commented 4 years ago

Looks like it's only useful for making the coverage job run as quick as the non-coverage ones

MetRonnie commented 4 years ago

I've put up a PR on this PR branch for only running in 4 processes when it's the coverage test https://github.com/oliver-sanders/isodatetime/pull/1

oliver-sanders commented 4 years ago

I've put up a PR on this PR branch for only running in 4 processes when it's the coverage test

Not sure why we would want to do that?

oliver-sanders commented 4 years ago

There wasn't much of a speedup because the slow test was monolithic so:

oliver-sanders commented 4 years ago

(will see what the performance looks like before disabling slow tests on selected platforms)

MetRonnie commented 4 years ago

Not sure why we would want to do that?

It did at least speed up the coverage run from 25 mins to 5 mins, but didn't seem to affect the others (also Python 3.7 and below seems to crash with it enabled). Not sure it's worth trying to improve 5 mins as that's pretty good I'd have thought

oliver-sanders commented 4 years ago

The Python<3.7 crash is due to Travis not installing the testing dependencies correctly, should be an easy fix.

Looks like the entire battery should now pass in ~5mins (15 for MacOS), takes ~35s on my box.

MetRonnie commented 4 years ago

Why is the coverage test back up to 25 minutes? My only other concern is that the log on Travis is now very long and it looks like it might be difficult to figure out which test(s) have failed.

oliver-sanders commented 4 years ago

Why is the coverage test back up to 25 minutes?

Dam, has that gone up? What was it before? Also note that Travis timings are not that stable.

MetRonnie commented 4 years ago
Strategy Before this PR First run of this PR Latest run of this PR
Ubuntu 5:30 4:30 4:30
Ubuntu + coverage 26:30 4:30** 25:00
Mac 19:30 15:00 15:30

**NOT TESTED

oliver-sanders commented 4 years ago

@MetRonnie I've batched the tests by century rather than year so there should only be 6 of them.

I did some local testing and it looks like coverage results in 3x slowdown, this factor seems to be higher on Actions.

Not much of a speedup on the coverage run sadly, looks like the thing taking the time is the coverage itself rather than the tests so multiprocessing doesn't help much.

The good news is the other runners are now running the full test battery (including the slow tests) faster than before.

MetRonnie commented 4 years ago

The coverage run is timing out 😬

I still think the way the logs are presented now is kinda annoying, just for a small speed boost in the non-coverage, non-Mac runs, it would be nicer if multiprocessing was turned off for them imho. But happy to approve either way.

MetRonnie commented 4 years ago

According to the docs, each Travis VM only has 2 CPUs, so I'd have thought this means 2 processes are more appropriate than 4