Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DPC-4434] Bump junit.jupiter to 5.11.3 #2354

Merged
merged 7 commits into from
Dec 16, 2024

Conversation

MEspositoE14s
Copy link
Contributor

🎫 Ticket

https://jira.cms.gov/browse/DPC-4434

🛠 Changes

Bump junit-jupiter to 5.11.3 to get our integration tests running again.

ℹ️ Context

When running make ci-app, both locally and as part of the Github build process our tests haven't been running. This was because the most recent bump of Drop Wizard brought in a transitive dependency on junit-jupiter 5.11.3, and we were running 5.10.2. This conflict doesn't cause a failure, but it does prevent Junit from finding any of our tests. This PR bumps our version to match the one pulled in from Drop Wizard and gets our tests running again.

Note

This change was merged on 11/26 and our tests haven't been running ever since. We're going to have to go back and fix any potential failing tests we've introduced since then.

🧪 Validation

  • Ran make ci-app locally and tests are running.
  • Submitted this PR and can see the tests failing.

@MEspositoE14s MEspositoE14s requested a review from a team December 13, 2024 16:50
@jdettmannnava
Copy link
Contributor

Aren't we going to need to fix the tests in order to merge?

@MEspositoE14s
Copy link
Contributor Author

Aren't we going to need to fix the tests in order to merge?

Yep, and in the meantime we should probably stop merging new code until it's done.

@@ -33,12 +34,12 @@ public SecretLoggingFilter() {
@Override
public Filter<ILoggingEvent> build() {
// Clean the secrets list
secrets.removeIf(secret -> !envVars.containsKey(secret));
List<String> containsSecrets = secrets.stream().filter(envVars::containsKey).collect(Collectors.toList());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still an improvement over the iterator!

@@ -14,7 +14,7 @@

<properties>
<mainClass>gov.cms.dpc.api.DPCAPIService</mainClass>
<jjwt.version>0.12.6</jjwt.version>
<jjwt.version>0.11.4</jjwt.version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverting this to the version we had before, as there's some signing key issues in JWTUnitTests that will take some refactoring to resolve

@lukey-luke
Copy link
Contributor

JJWT changes basically undo most of the changes from THIS PR. make ci-app is passing LGTM

@MEspositoE14s MEspositoE14s merged commit 3184fba into main Dec 16, 2024
8 checks passed
@MEspositoE14s MEspositoE14s deleted the me/dpc-4434-get-tests-running-again branch December 16, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants