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.

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

[fix][test] testModularLoadManagerRemoveBundleAndLoad #19710

Merged
merged 1 commit into from Mar 6, 2023
Merged

[fix][test] testModularLoadManagerRemoveBundleAndLoad #19710

merged 1 commit into from Mar 6, 2023

Conversation

AnonHxy
Copy link
Contributor

@AnonHxy AnonHxy commented Mar 4, 2023 •
edited

Motivation

Fixes #19656 (comment)

Modifications

Replacing ScheduledExecutorService to ExecutorService in testModularLoadManagerRemoveBundleAndLoad

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: AnonHxy#31

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 4, 2023
@AnonHxy AnonHxy mentioned this pull request Mar 4, 2023
Closed
@AnonHxy
Copy link
Contributor Author

AnonHxy commented Mar 4, 2023

@heesung-sn PTAL

@AnonHxy AnonHxy mentioned this pull request Mar 4, 2023
Merged
5 tasks
@AnonHxy AnonHxy self-assigned this Mar 4, 2023
tisonkun
tisonkun approved these changes Mar 4, 2023
View reviewed changes
Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

LGTM.

Is there any change we can move some cases or the test out of flaky tests? I guess we omit this test failure in #19656 because it's in flaky group :P

@tisonkun tisonkun changed the title [test][fix] Fix testModularLoadManagerRemoveBundleAndLoad failure [fix][test] testModularLoadManagerRemoveBundleAndLoad Mar 4, 2023
@tisonkun
Copy link
Member

tisonkun commented Mar 4, 2023

Flaky group tests pass but uploading Codecov failed. cc @yaalsn maybe you can take a look? https://github.com/apache/pulsar/actions/runs/4329101285/jobs/7559476284

@AnonHxy
Copy link
Contributor Author

AnonHxy commented Mar 6, 2023

LGTM.

Is there any change we can move some cases or the test out of flaky tests? I guess we omit this test failure in #19656 because it's in flaky group :P

It‘s not easy that we could always find the failure flaky tests. Maybe we could change all the flaky tests stable in a separate thread I think.

@AnonHxy AnonHxy merged commit 4ed8a87 into apache:master Mar 6, 2023
lhotari pushed a commit that referenced this pull request Mar 5, 2024
@AnonHxy @lhotari
[fix][test] testModularLoadManagerRemoveBundleAndLoad (#19710)
0cf5a83 
(cherry picked from commit 4ed8a87)

# Conflicts:
#	pulsar-broker/src/test/java/org/apache/pulsar/broker/namespace/NamespaceServiceTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tisonkun tisonkun tisonkun approved these changes

@coderzc coderzc coderzc approved these changes

@heesung-sn heesung-sn heesung-sn approved these changes

@poorbarcode poorbarcode Awaiting requested review from poorbarcode

Assignees

@AnonHxy AnonHxy

Labels
area/test doc-not-needed Your PR changes do not impact docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@AnonHxy @tisonkun @coderzc @heesung-sn