This message was deleted.

You’re mocking

PulsarAdmin

so you will have to mock the return value of

PulsarAdmin.tenants()

as well, just as you did for the clusters 😉

Thanks, it solved the null error

You basically have a chicken/egg situation here because

admin.tenants()

will return null until it is mocked to do otherwise. By default all mock methods that return a non-primitive value will return

null

. I believe in your case, if you just delete the

admin.tenants().createTenant("tenant1", TenantInfoImpl.builder()
                    .adminRoles(Set.of("role1","role2"))
                    .allowedClusters(Set.of(clusterName))
            .build());
    admin.tenants().createTenant("tenan2", TenantInfoImpl.builder()
            .adminRoles(Set.of("role1","role2"))
            .allowedClusters(Set.of(clusterName))
            .build());

everything else should work. You are spinning through the string list of tenants and mocking out the topics/namespaces for each tenant.

Yes you're right but the main reason of this was to test whether cluster will be deleted from tenant's allowed cluster info when we delete the cluster. This is the main issue and you can see the PR that I've opened as fix. Could you suggest something for this case?

Hi @Umut Bilal Okur sorry for the delay, on PTO so just on and offline a bit here and there but…. This works for the “not specify all argument case”

@Test
    public void testDeleteClusterWithoutAllOption() throws PulsarAdminException {

        // Enable chaining mock calls (eg. admin.tenants().getTenants())
        pulsarAdmin = mock(PulsarAdmin.class, RETURNS_DEEP_STUBS);
        clusters = mock(Clusters.class);
        when(pulsarAdmin.clusters()).thenReturn(clusters);
        cmdClusters = spy(new CmdClusters(() -> pulsarAdmin));

        String clusterName = "my-cluster";
        String otherClusterName = "other-cluster";
        List<String> tenants = Arrays.asList("tenant1", "tenant2");
        when(pulsarAdmin.tenants().getTenants()).thenReturn(tenants);
        when(pulsarAdmin.tenants().getTenantInfo("tenant1")).thenReturn(TenantInfoImpl.builder()
                .adminRoles(Set.of("role1"))
                .allowedClusters(new HashSet<>(Set.of(clusterName, otherClusterName)))
                .build());
        when(pulsarAdmin.tenants().getTenantInfo("tenant2")).thenReturn(TenantInfoImpl.builder()
                .adminRoles(Set.of("role1"))
                .allowedClusters(new HashSet<>(Set.of(clusterName, otherClusterName)))
                .build());

        // Invoke the delete command
        cmdClusters.run(new String[]{"delete", clusterName});

        // Verify the cluster was deleted
        verify(clusters).deleteCluster(clusterName);

        // Verify the cluster was removed from each tenants' set of allowedClusters
        TenantInfo modifiedTenant1 = TenantInfoImpl.builder()
                .adminRoles(Set.of("role1"))
                .allowedClusters(Set.of(otherClusterName))
                .build();
        verify(pulsarAdmin.tenants()).updateTenant(eq("tenant1"), eq(modifiedTenant1));

        TenantInfo modifiedTenant2 = TenantInfoImpl.builder()
                .adminRoles(Set.of("role1"))
                .allowedClusters(Set.of(otherClusterName))
                .build();
        verify(pulsarAdmin.tenants()).updateTenant(eq("tenant2"), eq(modifiedTenant2));
    }

I can tell you what I don’t like about this test though is that it is highly highly mocked out and tailored to the CmdClusters delete implementation for the “else” branch, namely this

Set<String> clusters = getAdmin().tenants().getTenantInfo(tenant).getAllowedClusters();
        clusters.remove(cluster);
        getAdmin().tenants().updateTenant(tenant, TenantInfo.builder()
                .adminRoles(getAdmin().tenants().getTenantInfo(tenant).getAdminRoles())
                .allowedClusters(clusters)
                .build());

        getAdmin().clusters().deleteCluster(cluster);

Other than doing this via an integration test, I am not sure of a better way w/o introducing some middle abstraction (aka DeleteService) which is out of the scope for the issue you are fixing. As you notice, there is not already a test for delete cluster in

TestCmdClusters

for this exact reason. I would go ahead and add something like the test above though so it at least verifies the cluster is removed from the TenantInfo.clusters set. You could add another test (something like

testDeleteClusterWithAllOption

) and that would require you to mock out the implementation for the “if delete all” path in the CmdClusters ⬇️

for (String namespace : getAdmin().namespaces().getNamespaces(tenant)) {
                        // Partitioned topic's schema must be deleted by deletePartitionedTopic()
                        // but not delete() for each partition
                        for (String topic : getAdmin().topics().getPartitionedTopicList(namespace)) {
                            getAdmin().topics().deletePartitionedTopic(topic, true, true);
                        }
                        for (String topic : getAdmin().topics().getList(namespace)) {
                            getAdmin().topics().delete(topic, true, true);
                        }
                        getAdmin().namespaces().deleteNamespace(namespace, true);
                    }
                    getAdmin().tenants().deleteTenant(tenant);

which is definiltey more involved and will get ugly quickly.

Thank you very much, it was extremely helpful! I will try to add the test with delete all option.