Skip to content

[BEAM-5887] Rollforward "Fix classifier for unshaded tests"#6868

Merged
kennknowles merged 1 commit intoapache:masterfrom
kennknowles:redo-unshaded
Oct 29, 2018
Merged

[BEAM-5887] Rollforward "Fix classifier for unshaded tests"#6868
kennknowles merged 1 commit intoapache:masterfrom
kennknowles:redo-unshaded

Conversation

@kennknowles
Copy link
Member

Rolling forward this fix, which we can test against postcommit without re-enabling parallel
builds.


Follow this checklist to help us incorporate your contribution quickly and easily:

  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

It will help us expedite review of your Pull Request if you tag someone (e.g. @username) to look at it.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status Build Status Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
Build Status --- --- ---

This reverts commit 808a7aa.

Rolling forward this fix, which we can test without re-enabling parallel
builds.
@kennknowles
Copy link
Member Author

Seed job has re-disabled parallel builds for postcommits, so going to run them against just this fix. This fix also speeds things up for local dev, so has value on its own.

@kennknowles
Copy link
Member Author

run java postcommit

@kennknowles
Copy link
Member Author

Also, notably #6864 only re-enable parallel builds for postcommit. That was an oversight on my part. If it had also re-enabled for precommit, it would likely have been caught there.

@kennknowles
Copy link
Member Author

R: @adude3141 @swegner

@kennknowles
Copy link
Member Author

OK, slightly more thinking and there's this: The failure was a unit test, not an integration test. It is run in both precommit and postcommit, but only postcommit failed. And only postcommit had enabled parallel builds. So I would guess that the classifier fix is OK.

@kennknowles
Copy link
Member Author

The test that failed was using Mockito argument capture across threads, and explicitly unsupported practice. It seems extremely likely that it was the parallel build that caused that test to start failing. Just another data point in moving forwards with parallel build efforts.

@kennknowles
Copy link
Member Author

Green including postcommit. Please take a look.

@adude3141
Copy link
Contributor

LGTM

@kennknowles kennknowles merged commit f968d56 into apache:master Oct 29, 2018
@kennknowles kennknowles deleted the redo-unshaded branch November 9, 2018 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments