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 lhotari, merlimat and codelipenghui 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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 7, 2022

equanz commented Jan 7, 2022

equanz commented Jan 11, 2022

hezhangjian commented Jan 11, 2022

equanz commented Jan 11, 2022 •

edited

Loading

equanz commented Jan 13, 2022

equanz commented Jan 14, 2022

equanz commented Jan 17, 2022

equanz commented Jan 19, 2022

hrsakai left a comment

BewareMyPower commented Jul 28, 2022

BewareMyPower commented Jul 28, 2022

mattisonchao commented Aug 10, 2022

equanz commented Aug 10, 2022 •

edited

Loading

mattisonchao commented Aug 11, 2022 •

edited

Loading

equanz commented Aug 15, 2022

mattisonchao commented Aug 16, 2022

equanz commented Aug 16, 2022

github-actions bot commented Sep 13, 2022

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

Conversation

equanz commented Jan 6, 2022 • edited Loading

Motivation

Modifications

Verifying this change

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

Documentation

equanz commented Jan 7, 2022

equanz commented Jan 7, 2022

equanz commented Jan 11, 2022

hezhangjian commented Jan 11, 2022

equanz commented Jan 11, 2022 • edited Loading

equanz commented Jan 13, 2022

equanz commented Jan 14, 2022

equanz commented Jan 17, 2022

equanz commented Jan 19, 2022

hrsakai left a comment

Choose a reason for hiding this comment

BewareMyPower commented Jul 28, 2022

BewareMyPower commented Jul 28, 2022

mattisonchao commented Aug 10, 2022

equanz commented Aug 10, 2022 • edited Loading

mattisonchao commented Aug 11, 2022 • edited Loading

equanz commented Aug 15, 2022

mattisonchao commented Aug 16, 2022

equanz commented Aug 16, 2022

github-actions bot commented Sep 13, 2022

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