Skip to content

Issue 507: Introduce @FlakyTest annotation for marking a few tests as flaky#504

Closed
sijie wants to merge 3 commits intoapache:masterfrom
sijie:mark_test_as_failures
Closed

Issue 507: Introduce @FlakyTest annotation for marking a few tests as flaky#504
sijie wants to merge 3 commits intoapache:masterfrom
sijie:mark_test_as_failures

Conversation

@sijie
Copy link
Copy Markdown
Member

@sijie sijie commented Sep 7, 2017

Descriptions of the changes in this PR:

  • introduced FlakyTest annotation
  • mark a few tests as flaky tests (create issues for them)

- introduced FlakyTest annotation
- mark a few tests as flaky tests (create issues for them)
@sijie
Copy link
Copy Markdown
Member Author

sijie commented Sep 7, 2017

It is WIP - try to use the precommit jenkins job debugging the flaky test.

(I'd like to bring the CI back to normal state as soon as possible)

@asfgit
Copy link
Copy Markdown

asfgit commented Sep 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/bookkeeper-precommit-pullrequest-docker/11/

@sijie
Copy link
Copy Markdown
Member Author

sijie commented Sep 7, 2017

retest this please

@asfgit
Copy link
Copy Markdown

asfgit commented Sep 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/bookkeeper-precommit-pullrequest-docker/12/

@sijie
Copy link
Copy Markdown
Member Author

sijie commented Sep 7, 2017

retest this please

@asfgit
Copy link
Copy Markdown

asfgit commented Sep 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/bookkeeper-precommit-pullrequest-docker/13/

readEntries.hasMoreElements();) {
LedgerEntry entry = readEntries.nextElement();
assertTrue(entry.data.getClass().getName(),
entry.data.getClass().getName().contains("PooledNonRetainedSlicedByteBuf"));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why are you removing this checks ? they are 'poor man' checks about the fact that we are pooling/non-pooling the ByteBuf. it is important to check that in v2 protocol bytebufs are "pooled"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this check is not correct, and depends heavily on the implementation of netty. because the class can be SimpleLeakAwareByteBuf.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sijie OK I am convinced.
It is a pity that we are not able to made a usable assertion :-(

the error is
java.lang.AssertionError: io.netty.buffer.SimpleLeakAwareByteBuf
at org.apache.bookkeeper.client.BookKeeperTest.testReadEntryReleaseByteBufs(BookKeeperTest.java:742)

@sijie
Copy link
Copy Markdown
Member Author

sijie commented Sep 7, 2017

retest this please

@asfgit
Copy link
Copy Markdown

asfgit commented Sep 7, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/bookkeeper-precommit-pullrequest-docker/15/

@sijie
Copy link
Copy Markdown
Member Author

sijie commented Sep 9, 2017

retest this please

@asfgit
Copy link
Copy Markdown

asfgit commented Sep 9, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/bookkeeper-precommit-pullrequest-docker/16/

@sijie sijie changed the title Investigate CompactionByEntriesTest issue (WIP) Issue 507: Introduce @FlakyTest annotation for marking a few tests as flaky Sep 12, 2017
@sijie
Copy link
Copy Markdown
Member Author

sijie commented Sep 12, 2017

removed the debug logging (was unable to reproduce the failed test).

turn this pull request to introduce @FlakyTest annotation for disabling and tracking the flaky tests

@asfgit
Copy link
Copy Markdown

asfgit commented Sep 12, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/bookkeeper-precommit-pullrequest-docker/18/

Copy link
Copy Markdown
Member

@jiazhai jiazhai left a comment

Choose a reason for hiding this comment

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

+1. LGTM.

@jiazhai jiazhai added this to the 4.6.0 milestone Sep 13, 2017
@jiazhai jiazhai closed this in 131d127 Sep 13, 2017
ivankelly pushed a commit to ivankelly/bookkeeper that referenced this pull request Nov 9, 2017
…ests as flaky

Descriptions of the changes in this PR:

- introduced FlakyTest annotation
- mark a few tests as flaky tests (create issues for them)

Author: Sijie Guo <sijie@apache.org>

Reviewers: Jia Zhai <None>

This closes apache#504 from sijie/mark_test_as_failures, closes apache#507
jiazhai pushed a commit that referenced this pull request Nov 10, 2017
…s flaky

Descriptions of the changes in this PR:

- introduced FlakyTest annotation
- mark a few tests as flaky tests (create issues for them)

Author: Sijie Guo <sijieapache.org>

Reviewers: Jia Zhai <None>

This closes #504 from sijie/mark_test_as_failures, closes #507

Descriptions of the changes in this PR:

(PR description content here)...

Author: Sijie Guo <sijie@apache.org>

Reviewers: Enrico Olivelli <eolivelli@gmail.com>, Jia Zhai <None>, Sijie Guo <sijie@apache.org>

This closes #710 from ivankelly/flakey-4.5, closes #507
@sijie sijie deleted the mark_test_as_failures branch July 16, 2018 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants