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

ArchUnit refactore code, fix tests and test imports #3713

Open
lorriborri opened this issue Dec 6, 2024 · 2 comments · May be fixed by #3778
Open

ArchUnit refactore code, fix tests and test imports #3713

lorriborri opened this issue Dec 6, 2024 · 2 comments · May be fixed by #3778
Labels
bug Something isn't working enhancement New feature or request

Comments

@lorriborri
Copy link
Member

lorriborri commented Dec 6, 2024

Situation

The ArchUnit tests are flaky, because they wok on the build level. While e.g. the eclipse build builds the whole project (test classes and main classes), gradle and intelliJ build, builds only the dependent classes. Further some tests seem not to work properly.

Wanted

Working ArchUnit tests wihtout side effects on different build tools.

Solution

  1. We need to fix the resideInPackage with ..test.. (no working properly)
  2. We need to fix the execution of the unit tests, test classes are only available when tests were executed before executing the archunit tests:
    Solution 1: write ArchUnitExtension annotation for Junit test,
    test are only executed when sechub.archunit.buildfolder is set,
    GH action: ./gradlew :sechub-archunit-test:test after build,
    add shell command to sdc.sh
    Solution 2: we might can fix this by adjusting the gradle build execution order and dependencies
    Solution 3: There might be an archunit gradle plugin?
  3. We need to document if side effects still exists (e.g. with .because, when to use "sechub.archunit.ignoreFolders")
@lorriborri lorriborri added bug Something isn't working enhancement New feature or request labels Dec 6, 2024
@lorriborri lorriborri changed the title ArchUnit fix tests and imports ArchUnit refactore code, fix tests and test imports Dec 6, 2024
@de-jcup
Copy link
Member

de-jcup commented Dec 10, 2024

Here a good example how to test this:

An eclipse build does always compile all classes (also test classes etc.) and when we start the arch unit tests with eclipse we have (at least one) failing test(s) - e.g. CodingRulesTest complains about:

java.lang.AssertionError: Architecture Violation [Priority: MEDIUM] - Rule 'no classes should access @Deprecated members, because there should be a better alternative' was violated (3 times):
Method <com.mercedesbenz.sechub.domain.scan.product.sereco.SerecoReportProductExecutor.$SWITCH_TABLE$com$mercedesbenz$sechub$sharedkernel$ProductIdentifier()> gets field <com.mercedesbenz.sechub.sharedkernel.ProductIdentifier.NESSUS> in (SerecoReportProductExecutor.java:35)
Method <com.mercedesbenz.sechub.domain.scan.product.sereco.SerecoReportProductExecutor.$SWITCH_TABLE$com$mercedesbenz$sechub$sharedkernel$ProductIdentifier()> gets field <com.mercedesbenz.sechub.sharedkernel.ProductIdentifier.NETSPARKER> in (SerecoReportProductExecutor.java:35)
Method <com.mercedesbenz.sechub.storage.s3.AbstractAwsS3Storage$LogS3ProgressListener.$SWITCH_TABLE$com$amazonaws$event$ProgressEventType()> gets field <com.amazonaws.event.ProgressEventType.BYTE_TRANSFER_EVENT> in (AbstractAwsS3Storage.java:277)
  at com.tngtech.archunit.lang.ArchRule$Assertions.assertNoViolation(ArchRule.java:94)
  at com.tngtech.archunit.lang.ArchRule$Assertions.check(ArchRule.java:86)
  at com.tngtech.archunit.lang.ArchRule$Factory$SimpleArchRule.check(ArchRule.java:165)

Which is correct, because we marked the product identifiers "NESSUS" and "NETSPARKER" as deprecated and did not ignore those.

But the CI/CD build does NOT fail. Reason is - as mentioned already before in issue description - that at gradle builds not all classes are compiled before they are tested by arch unit.

We could use this test as a canary that the changes, which will be done inside this issue, work. When it works the test will fail in gradle as well. Aftwards we should simply remove the deprecation parts from the product identifiers (because they are still in use by Sereco) or we add a ignore for this situation.

@lorriborri
Copy link
Member Author

lorriborri added a commit that referenced this issue Jan 8, 2025
- not tested yet
- rely on project and test dependencies
- arch unit executed after all others
@lorriborri lorriborri linked a pull request Jan 8, 2025 that will close this issue
lorriborri added a commit that referenced this issue Jan 8, 2025
lorriborri added a commit that referenced this issue Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants