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

Make sure Konsist tests always run #1590

Merged
merged 2 commits into from
Oct 18, 2023

Conversation

jmartinesp
Copy link
Contributor

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

This PR adds a workaround so Konsist tests are always run.

Motivation and context

Konsist tests weren't being run in either the CI or locally using gradle because all the tests were considered up to date. By setting the output as never up to date we make sure they're always run.

Tests

  • If the tests fail with the broken test commit, it's working.

@jmartinesp jmartinesp requested a review from a team as a code owner October 17, 2023 10:34
@jmartinesp jmartinesp requested review from ganfra and removed request for a team October 17, 2023 10:34
build.gradle.kts Outdated Show resolved Hide resolved
@jmartinesp jmartinesp force-pushed the misc/jme/fix-konsist-tests-not-running-on-check branch from f410311 to cb7e6d3 Compare October 17, 2023 10:43
@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2023

📱 Scan the QR code below to install the build (arm64 only) for this PR.
QR code
If you can't scan the QR code you can install the build via this link: https://i.diawi.com/MUMzfy

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Thanks, please see my comment. Once the PR has been cleaned up, it can be merged!

// Make sure Konsist tests are always run
tasks.withType<Test>().configureEach {
outputs.upToDateWhen { false }
}
Copy link
Member

@bmarty bmarty Oct 18, 2023

Choose a reason for hiding this comment

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

Konsist test are not always run because the konsist module does not depend on any other modules, except projects.libraries.architecture and projects.libraries.designsystem (see the lines above) because we need access to some external types.
Maybe add this in the comment above to explain why this patch is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in fb0bf38.

@jmartinesp
Copy link
Contributor Author

jmartinesp commented Oct 18, 2023

Added some changes to make sure Konsist tests are only run (and fail) in one flow, I'll remove the offending code later.

EDIT: it works! Removing the commit that causes Konsist failures.

@jmartinesp jmartinesp force-pushed the misc/jme/fix-konsist-tests-not-running-on-check branch from fb0bf38 to 5b404c2 Compare October 18, 2023 08:45
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Just a quick update of the comment, else LGTM

tests/konsist/build.gradle.kts Outdated Show resolved Hide resolved
Co-authored-by: Benoit Marty <benoit@matrix.org>
@jmartinesp jmartinesp enabled auto-merge (squash) October 18, 2023 09:11
@sonarcloud
Copy link

sonarcloud bot commented Oct 18, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@jmartinesp jmartinesp merged commit e5a8fd9 into develop Oct 18, 2023
13 checks passed
@jmartinesp jmartinesp deleted the misc/jme/fix-konsist-tests-not-running-on-check branch October 18, 2023 09:41
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.

None yet

2 participants