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

Remove sharding on functional tests #2104

Closed
wants to merge 1 commit into from

Conversation

ssbarnea
Copy link
Member

Sharding tests was hacky approach for dealing with Travis build time
limitations, one that caused lots of issues.

In order to simplify the testing and ease the migration toward other
CI system, we are better-off dropping the sharding feature (which also
seems to prevent bumping pytest).

@ssbarnea ssbarnea added the test Improvement to quality assurance: CI/CD, testing, building label Jun 15, 2019
@ssbarnea ssbarnea self-assigned this Jun 15, 2019
* Remove sharding on functional tests

Sharding tests was hacky approach for dealing with Travis build time
limitations, one that caused lots of issues.

In order to simplify the testing and ease the migration toward other
CI system, we are better-off dropping the sharding feature (which also
seems to prevent bumping pytest).

* Remove most YAML anchors which prevented use of official syntax for
travis file for "env" blocks (lists not dictionaries).

* Consolidated newlines between jobs (readability)

Note: This is more of a POC change as we will need other optimizations
in order to assure that functional always runs under 50min time limit.

Signed-off-by: Sorin Sbarnea <ssbarnea@redhat.com>
@themr0c
Copy link
Contributor

themr0c commented Jun 18, 2019

Great, but we really can't do that before the need to have builds running on Travis has disappeared, because all builds on Travis will fail by design.

@ssbarnea
Copy link
Member Author

That is true and this is why it was marked as a draft. Still, there is something that could potentially make this real: if we manage to optimize the execution time. Currently functional takes around 60min and Travis limit is 50mins. If we cut it to ~40mins we can make it real (we need to have ~10min safety net).

@decentral1se

This comment has been minimized.

@decentral1se
Copy link
Contributor

decentral1se commented Jul 2, 2019

Ok, new idea.

Since #2139 and #2137 show that we can run a whole bunch of Molecule in parallel I propose that we actually move forward with this attempt. We remove sharding altogether.

For the functional test suite, I propose a marker (-m parallel, -m not parallel) to split the test suite into smaller pieces to be run serially or in parallel. We configure Travis to run the functional test suite twice (with both markers, currently the best-in-class solution ala pytest-dev/pytest-xdist#385). The unit tests will already be run with -n auto

EDIT: Further clarification: instead of running each functional job with shard 1, 2, 3 we would instead run each functional job against -m parallel and -m not parallel (so, two jobs).

This might work! Then I can finally get on with #1811 and ride into the sunset ...

decentral1se added a commit to decentral1se/molecule that referenced this pull request Jul 2, 2019
Showing a smaller diff than ansible#2104
@decentral1se
Copy link
Contributor

Hey, take a look at decentral1se@cdf902c. That's a much smaller diff with less unrelated changes for removing sharding. If we merged #2139 and #2137 then I could work from this sharding diff to refactor the Travis configuration to use -m parallel/not parallel.

@decentral1se
Copy link
Contributor

FYI, plan has changed now since further experimentation. There are 200+ functional tests which must be run in serial execution mode. So, we can't afford to remove sharding even if we're running some tests in parallel mode (~25 tests only!). So, let's close this off?

@ssbarnea
Copy link
Member Author

ssbarnea commented Aug 4, 2019

Closing because is not doable yet.

@ssbarnea ssbarnea closed this Aug 4, 2019
@ssbarnea ssbarnea deleted the fix/desharding branch October 19, 2019 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Improvement to quality assurance: CI/CD, testing, building
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants