Skip to content

parallelize vibe.d tests #98

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.

Sign up for GitHub

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

Jump to bottom
Merged
dlang-bot merged 2 commits into from
Jan 25, 2018
Merged

parallelize vibe.d tests #98

dlang-bot merged 2 commits into from
Jan 25, 2018

Conversation

@MartinNowak
Copy link
Member

@MartinNowak MartinNowak commented Dec 21, 2017

@dlang-bot
Copy link
Collaborator

dlang-bot commented Dec 21, 2017

Thanks for your pull request, @MartinNowak!

@MartinNowak MartinNowak force-pushed the parallel_vibe_tests branch 7 times, most recently from 8680377 to 79c05b4 Compare December 21, 2017 22:49
wilzbach
wilzbach reviewed Dec 22, 2017
View reviewed changes
vars/runPipeline.groovy
"rejectedsoftware/vibe.d",
"vibe-d/vibe.d+libevent-base",
"vibe-d/vibe.d+libevent-examples",
"vibe-d/vibe.d+libevent-tests",
Copy link
Contributor

@wilzbach wilzbach Dec 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vibe-core is the new default with 0.8.3, so it might be worth a consideration to drop these tests as libevent is on its way out.
After all, they are really expensive and the goal of the project tester is to avoid that we break projects, not add another CI for them.
See: vibe-d/vibe.d#2002

Copy link
Member Author

@MartinNowak MartinNowak Dec 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's quite an important project, so it seems better we cover all the supported code.

vars/runPipeline.groovy
"vibe-d/vibe.d+vibe-core-base",
"vibe-d/vibe.d+vibe-core-examples",
"vibe-d/vibe.d+vibe-core-tests",
"vibe-d/vibe.d+libasync-base",
Copy link
Contributor

@wilzbach wilzbach Dec 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could consider dropping this too. At least until event core supports it...

wilzbach
wilzbach reviewed Dec 27, 2017
View reviewed changes
vars/runPipeline.groovy Outdated
if (repo == 'vibe-d/vibe.d') {
// https://github.com/vibe-d/vibe.d/pull/2016
// clone("https://github.com/${repo}.git", 'v0.8.3-alpha.1')
clone('https://github.com/MartinNowak/vibe.d.git', 'parallel_tests')
Copy link
Contributor

@wilzbach wilzbach Dec 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use upstream now, e.g. by cloning commit 5766e5135b77ab62da21e29ed0660ac9178ea093

Copy link
Member Author

@MartinNowak MartinNowak Dec 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately Jenkins GitSCM checkout doesn't seem to support commit hashes.

@MartinNowak MartinNowak force-pushed the parallel_vibe_tests branch 3 times, most recently from 6cc494f to eef6633 Compare December 28, 2017 18:23
@MartinNowak
Copy link
Member Author

MartinNowak commented Jan 4, 2018

Looks like this unveiled a dub bug introduced in dlang/dub#1263, will have to track that down first.

@wilzbach wilzbach force-pushed the parallel_vibe_tests branch from c1eeadb to a40e9e5 Compare January 25, 2018 10:22
@wilzbach
Copy link
Contributor

wilzbach commented Jan 25, 2018

Removed the special casing of Vibe.d as 0.8.3-alpha.3 is now available and rebased to upstream/master (without conflicts).

@MartinNowak
Copy link
Member Author

MartinNowak commented Jan 25, 2018

Dub bug has been fixed in dlang/dub#1350.

@wilzbach
Copy link
Contributor

wilzbach commented Jan 25, 2018

Looks like there's a new failure coming from of Vibe.d :/
I assume that this is due to Vibe.d switching to using vibe-core by default with the upcoming 0.8.3

7E3589D5(----) INF] Receive loop finished.
Receiver watchdog failure.
Program exited with code 1
script returned exit code 2

@wilzbach wilzbach force-pushed the parallel_vibe_tests branch from a40e9e5 to cca3fe3 Compare January 25, 2018 11:07
@wilzbach
Copy link
Contributor

wilzbach commented Jan 25, 2018

I assume that this is due to Vibe.d switching to using vibe-core by default with the upcoming 0.8.3

Or latest tags simply ignores the alpha versions:

 > git rev-parse v0.8.2^{commit} # timeout=10
Checking out Revision 38d5da2bd87595cf4c9c6c744301d3ee9300e730 (v0.8.2)

(updated the groovy script to an explicit checkout)

@dlang-bot dlang-bot merged commit e6c6638 into dlang:master Jan 25, 2018
@wilzbach
Copy link
Contributor

wilzbach commented Jan 25, 2018

Nice the total time went down from 33m39s to 12m13s!!! Nice job!
For convenience, the run time per component:

component time
vibe.d+libasync-base 3m45s
vibe.d+libevent-base 4m20s
vibe.d+libevent-examples 12m1s
vibe.d+libevent-tests 8m35s
vibe.d+vibe-core-base 4m31s
vibe.d+vibe-core-examples 9m51s
vibe.d+vibe-core-tests 6m44s

@MartinNowak MartinNowak deleted the parallel_vibe_tests branch January 25, 2018 11:29
@MartinNowak
Copy link
Member Author

MartinNowak commented Jan 25, 2018

Thanks, some of my dub work should significantly reduce the necessary time.

@MartinNowak
Copy link
Member Author

MartinNowak commented Jan 25, 2018

Or latest tags simply ignores the alpha versions

Yes, the regex only matches release versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@wilzbach wilzbach wilzbach left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

auto-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MartinNowak @dlang-bot @wilzbach