fabric8io / kubernetes-client

Java client for Kubernetes & OpenShift
http://fabric8.io
Apache License 2.0
3.43k stars 1.46k forks source link

Use JUnit5 extensions instead of deprecated Junit4 Rules / Remove `EnableRuleMigrationSupport` #2849

Closed rohanKanojia closed 3 years ago

rohanKanojia commented 3 years ago

Right now in kubernetes-tests/ module, we're using @EnableRuleMigrationSupport annotation to allow usage of Junit4 rules and we're using KubernetesServer @Rule for Kubernetes Mock Server that is used in each test: https://github.com/fabric8io/kubernetes-client/blob/33a735a61a121083e4dfd0e9ba9105e62936cca6/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ConfigMapTest.java#L34-L38

Due to this, we're also dependent on junit-jupiter-migrationsupport:

https://github.com/fabric8io/kubernetes-client/blob/33a735a61a121083e4dfd0e9ba9105e62936cca6/kubernetes-tests/pom.xml#L47-L51

I added Junit5 extensions for Kubernetes Mock Server (EnableKubernetesMockClient for Kubernetes and EnableOpenShiftMockClient) in https://github.com/fabric8io/kubernetes-client/pull/2785 which can be used to initialize both KubernetesMockServer and KubernetesClient. So one can easily refactor the above test to use Junit5 extension annotation like this:

@EnableKubernetesMockClient
class ConfigMapTest {
  KubernetesMockServer server; // initialized via above annotation
  KubernetesClient client;     // initialized via above annotation

  @Test
  void testLiteralConfigMap() throws InterruptedException {
     // ...
  }

  @Test
  void testFromResourceWithFileConfigMap() throws InterruptedException {
     // ...
  }

  @Test
  void testFromResourceConfigMap() throws InterruptedException {
     // ...
  }
}

In case anyone is interested, it would be nice if we could move from Junit4 Rules to new Junit5 extension annotations and get rid of this dependency as well. You can easily find occurrences of EnableRuleMigrationSupport annotation with a simple grep like this:

git grep -c "EnableRuleMigrationSupport"
harsh1599 commented 3 years ago

Would love to work on this, thanks!

rohanKanojia commented 3 years ago

Although most of the tests were updated to get rid of this annotation. I still see some tests in extensions/ and uberjar/ module using this annotation. We should apply the same fix in these modules too

kubernetes-client : $ git grep -inc "EnableRuleMigrationSupport"
extensions/camel-k/tests/src/test/java/io/fabric8/camelk/test/crud/IntegrationCrudTest.java:2
extensions/chaosmesh/tests/src/test/java/io/fabric8/chaosmesh/test/AdaptTest.java:2
extensions/chaosmesh/tests/src/test/java/io/fabric8/chaosmesh/test/crud/IoChaosTest.java:2
extensions/knative/tests/src/test/java/io/fabric8/knative/test/EventsContribResourcesTest.java:2
extensions/knative/tests/src/test/java/io/fabric8/knative/test/KnativeSourcesApiGroupResourcesTest.java:2
extensions/knative/tests/src/test/java/io/fabric8/knative/test/RouteTest.java:2
extensions/knative/tests/src/test/java/io/fabric8/knative/test/ServiceTest.java:2
extensions/knative/tests/src/test/java/io/fabric8/knative/test/crud/RouteCrudTest.java:2
extensions/knative/tests/src/test/java/io/fabric8/knative/test/crud/ServiceCrudTest.java:2
extensions/service-catalog/tests/src/test/java/io/fabric8/servicecatalog/test/AdaptTest.java:2
extensions/service-catalog/tests/src/test/java/io/fabric8/servicecatalog/test/crud/ClusterServiceBrokerTest.java:2
extensions/service-catalog/tests/src/test/java/io/fabric8/servicecatalog/test/crud/ClusterServiceClassTest.java:2
extensions/service-catalog/tests/src/test/java/io/fabric8/servicecatalog/test/crud/ClusterServicePlanTest.java:2
extensions/service-catalog/tests/src/test/java/io/fabric8/servicecatalog/test/crud/ServiceBindingTest.java:2
extensions/service-catalog/tests/src/test/java/io/fabric8/servicecatalog/test/crud/ServiceInstanceTest.java:2
extensions/tekton/tests/src/test/java/io/fabric8/tekton/pipeline/v1beta1/PipelineTest.java:2
extensions/tekton/tests/src/test/java/io/fabric8/tekton/test/crud/PipelineCrudTest.java:2
extensions/tekton/tests/src/test/java/io/fabric8/tekton/test/crud/V1alpha1PipelineCrudTest.java:2
extensions/tekton/tests/src/test/java/io/fabric8/tekton/triggers/v1alpha1/EventListenerTest.java:2
extensions/tekton/tests/src/test/java/io/fabric8/tekton/triggers/v1alpha1/TriggerBindingTest.java:2
extensions/tekton/tests/src/test/java/io/fabric8/tekton/triggers/v1alpha1/TriggerTemplateTest.java:2
extensions/volumesnapshot/tests/src/test/java/io/fabric8/volumesnapshot/test/AdaptTest.java:2
extensions/volumesnapshot/tests/src/test/java/io/fabric8/volumesnapshot/test/crud/VolumeSnapshotClassTest.java:2
extensions/volumesnapshot/tests/src/test/java/io/fabric8/volumesnapshot/test/crud/VolumeSnapshotTest.java:2
uberjar/src/test/java/io/fabric8/kubernetes/clnt/UberJarTest.java:2
rohanKanojia commented 3 years ago

We would also need to implement Junit5 extensions for mocks for extensions (see example here)

MUzairS15 commented 3 years ago

Pls assign me .