apache / accumulo-proxy

Apache Accumulo Proxy
https://accumulo.apache.org
Apache License 2.0
9 stars 19 forks source link

Junit update to JUnit5 #38

Closed DomGarguilo closed 1 year ago

DomGarguilo commented 1 year ago

Closes #34

This PR:

With these changes, the test failure described in #37 still fails. These changes also break ProxyDurabilityIT.testDurability() because it relies on ConfigurableMacBase from accumulo and the mismatch in JUnit versions between 2.0.x (using JUnit4) and JUnit5 in these changes breaks things. If this is merged, I will create a new ticket to track that test failure which I think will be fixed on it own when the project is moved to use accumulo 2.1.

DomGarguilo commented 1 year ago

The changes seem okay to me, but the tests are still failing.

The compaction related test was failing prior to these changes. The one other failing test is due to a mismatch in JUnit versions between JUnit5 on this branch and JUnit4 in accumulo 2.0 which causes issues when using certain methods from ConfigurableMacBase, in this case getClientProperties() is causing issues.

ctubbsii commented 1 year ago

The compaction related test was failing prior to these changes. The one other failing test is due to a mismatch in JUnit versions between JUnit5 on this branch and JUnit4 in accumulo 2.0

Okay, so what is needed to fix these? Do you need to merge this now, and work on those as follow-on?

DomGarguilo commented 1 year ago

The compaction related test was failing prior to these changes. The one other failing test is due to a mismatch in JUnit versions between JUnit5 on this branch and JUnit4 in accumulo 2.0

Okay, so what is needed to fix these? Do you need to merge this now, and work on those as follow-on?

I think that might be a good option. The compaction tests will need to be updated anyways so can be fixed when moving to 2.1 and I assume that the other test failure will be resolved once the JUnit versions match between MAC and the rest for the test.