Rename test file name from `Test2.java` to `Test.java` to run all tests correctly #13644

equanz · 2022-01-06T07:38:38Z

Motivation

It seems some test cases like **/*Test2.java aren't run on the default test lifecycle.
This issue is caused by #10148. I'd like to fix it.

Modifications

Rename test file name to **/*Test.java
Fix some codes to follow changes

Verifying this change

Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (no)
The public API: (no)
The schema: (no)
The default values of configurations: (no)
The wire protocol: (no)
The rest endpoints: (no)
The admin cli options: (no)
Anything that affects deployment: (no)

Documentation

no-need-doc

equanz · 2022-01-07T00:18:07Z

/pulsarbot run-failure-checks

equanz · 2022-01-07T02:29:18Z

/pulsarbot run-failure-checks

equanz · 2022-01-11T02:44:04Z

@lhotari Could you please review this PR?

hezhangjian · 2022-01-11T07:03:15Z

I am a bit confused. It looks like your change code out of test scope, could you please explain why change NamespaceBases and delete method testDistinguishTopicTypeWhenForceDeleteNamespace? Thanks :)

equanz · 2022-01-11T08:54:44Z

why change NamespacesBase

After enabling *Test2.java cases, I found some bugs. This PR includes these fixes.

For Refactored PulsarResources to avoid building metadata paths everywhere #11693 changes, can't run org.apache.pulsar.broker.admin.v1.V1_AdminApi2Test#namespaceAntiAffinity correctly. Because org.apache.pulsar.broker.admin.AdminResource#namespaceName is not set from {cluster}/antiAffinity/{group} endpoint.
- https://github.com/equanz/pulsar/blob/2da21db57977715e21cc8155a5b31c09a3b0e967/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java#L1891
  - I'll consider to modify validateNamespacePolicyOperation in this method (and add some validation)
Currently, can't run org.apache.pulsar.broker.admin.AdminApi2Test#testForceDeleteNamespace correctly. In current code, try to delete topics and namespace bundles simultaneously. However, broker can't close (through unloading) and delete persistent topic simultaneously because of isClosingOrDeleting field.
- https://github.com/equanz/pulsar/blob/2da21db57977715e21cc8155a5b31c09a3b0e967/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java#L464-L482

why delete method testDistinguishTopicTypeWhenForceDeleteNamespace

For #12663 changes, can't put invalid path like

pulsar/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiTest2.java

Lines 1597 to 1599 in 793dd91

    
           // insert an invalid topic name 
        
           pulsar.getLocalMetadataStore().put( 
        
                   "/managed-ledgers/" + exNs + "/persistent/", "".getBytes(), Optional.empty()).join();

.
I wasn't sure this case was still needed, so I deleted it.
If we should run testDistinguishTopicTypeWhenForceDeleteNamespace case, use some method instead of put.

equanz · 2022-01-13T08:06:05Z

/pulsarbot run-failure-checks

equanz · 2022-01-14T05:43:35Z

/pulsarbot run-failure-checks

equanz · 2022-01-17T01:07:00Z

/pulsarbot run-failure-checks

equanz · 2022-01-19T04:44:41Z

@shoothzj Could you check the comment #13644 (comment) please?

hrsakai

LGTM

To resolve the conflicts brought by #13644

BewareMyPower · 2022-07-28T15:58:38Z

Add release/2.9.4 and release/2.10.2 labels for this PR, see #16854.

BewareMyPower · 2022-07-28T16:06:50Z

Remove the release/2.10.2 label because it's already cherry-picked into branch-2.10

mattisonchao · 2022-08-10T12:43:19Z

Hi @equanz

It looks like we have many conflicts with branch-2.9. Could you help cherry-pick this PR to branch-2.9?

equanz · 2022-08-10T13:12:59Z

@mattisonchao
Okay. I'll check it.
So, what should I do? (create a new PR to branch-2.9, check some PRs, etc.)

…ests correctly (apache#13644) * test: rename test class * fix: fix to follow existing test cases * test: fix to follow existing production codes

mattisonchao · 2022-08-11T02:14:06Z

Hi @equanz
I've got your new PR, thank you a lot!

equanz · 2022-08-15T23:24:58Z

@mattisonchao

How is it going? As you know, I created the new PR #17048 .

mattisonchao · 2022-08-16T01:34:40Z

@equanz
I will review it today. thanks!

equanz · 2022-08-16T01:53:18Z

I got it. Thank you for maintaining branches.

github-actions · 2022-09-13T03:19:29Z

@equanz Please provide a correct documentation label for your PR.
Instructions see Pulsar Documentation Label Guide.

github-actions bot assigned equanz Jan 6, 2022

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 6, 2022

equanz force-pushed the fix_to_run_all_tests branch from 4516e32 to 2da21db Compare January 6, 2022 09:09

equanz force-pushed the fix_to_run_all_tests branch from 2da21db to f7a8a10 Compare January 12, 2022 08:44

equanz added 3 commits January 19, 2022 11:15

test: rename test class

e767b4c

fix: fix to follow existing test cases

1150b7f

test: fix to follow existing production codes

446ef2b

equanz force-pushed the fix_to_run_all_tests branch from f7a8a10 to 446ef2b Compare January 19, 2022 02:47

hezhangjian approved these changes Jan 20, 2022

View reviewed changes

hezhangjian requested review from codelipenghui, lhotari and merlimat January 20, 2022 02:46

hrsakai approved these changes Jan 20, 2022

View reviewed changes

hrsakai merged commit 96e98fc into apache:master Jan 20, 2022

equanz deleted the fix_to_run_all_tests branch January 20, 2022 06:42

equanz mentioned this pull request Jan 25, 2022

Add new tenant operation LIST_ANTI_AFFINITY_NAMESPACES and fix validation of getAntiAffinityNamespaces #13723

Open

2 tasks

liangyepianzhou mentioned this pull request Mar 10, 2022

Got 500 while force delete namespace #13793

Closed

poorbarcode mentioned this pull request Apr 7, 2022

[Test] org.apache.pulsar.broker.admin.AdminApi2Test in branch-2.9 have multiple failed tests #14863

Closed

BewareMyPower added a commit that referenced this pull request Jul 27, 2022

Rename test file name from *Test2.java to *Test.java

ad868ac

To resolve the conflicts brought by #13644

BewareMyPower added area/test release/2.10.2 labels Jul 28, 2022

BewareMyPower added the release/2.9.4 label Jul 28, 2022

BewareMyPower mentioned this pull request Jul 28, 2022

[branch-2.9] Some broken tests are ignored in CI #16854

Closed

BewareMyPower removed the release/2.10.2 label Jul 28, 2022

sijie mentioned this pull request Jul 28, 2022

ISSUE-16854: [branch-2.9] Some broken tests are ignored in CI streamnative/pulsar-archived#4639

Open

equanz mentioned this pull request Aug 10, 2022

[branch-2.9] Rename test file name from *Test2.java to *Test.java to run all tests correctly #17048

Merged

2 tasks

mattisonchao added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Sep 13, 2022

github-actions bot added doc-label-missing and removed doc-not-needed Your PR changes do not impact docs labels Sep 13, 2022

equanz mentioned this pull request Nov 7, 2023

[fix][test] Fix that some test cases are not running as expected on CI #21532

Merged

15 tasks

Rename test file name from *Test2.java to *Test.java to run all tests correctly #13644

Rename test file name from *Test2.java to *Test.java to run all tests correctly #13644

Uh oh!

Conversation

equanz commented Jan 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

equanz commented Jan 7, 2022

Uh oh!

equanz commented Jan 7, 2022

Uh oh!

equanz commented Jan 11, 2022

Uh oh!

hezhangjian commented Jan 11, 2022

Uh oh!

equanz commented Jan 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

equanz commented Jan 13, 2022

Uh oh!

equanz commented Jan 14, 2022

Uh oh!

equanz commented Jan 17, 2022

Uh oh!

equanz commented Jan 19, 2022

Uh oh!

hrsakai left a comment

Choose a reason for hiding this comment

Uh oh!

BewareMyPower commented Jul 28, 2022

Uh oh!

BewareMyPower commented Jul 28, 2022

Uh oh!

mattisonchao commented Aug 10, 2022

Uh oh!

equanz commented Aug 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattisonchao commented Aug 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

equanz commented Aug 15, 2022

Uh oh!

mattisonchao commented Aug 16, 2022

Uh oh!

equanz commented Aug 16, 2022

Uh oh!

github-actions bot commented Sep 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Rename test file name from `Test2.java` to `Test.java` to run all tests correctly #13644

Rename test file name from `Test2.java` to `Test.java` to run all tests correctly #13644

equanz commented Jan 6, 2022 •

edited

Loading

equanz commented Jan 11, 2022 •

edited

Loading

equanz commented Aug 10, 2022 •

edited

Loading

mattisonchao commented Aug 11, 2022 •

edited

Loading