Tweak noncompilation tests and add MiMa exclusions #98
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The MUnit `compileErrors` macro seems to follow a different compilation cadence than other code, meaning the tests are more likely to erroneously fail. This seems to work better.
atoivonen
approved these changes
Oct 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Test Changes
There is currently an issue in this repo that can cause the Scala 3 tests (and prior to this PR, the Scala 2.13 tests) to falsely fail. Essentially it's a compilation order issue; when the tests use macros to compile example strings in the test, the tests must be compiled after the generated Smithy4s code, as-modified by our Scalafix.
In other words, if you run in sbt
the tests will fail, because the Smithy4s code is generated and modified in the following order:
Test/compileis triggered, which depends onCompile/compile.Compile/compileis therefore triggered, which depends on Smithy4s codegen.Compile/compileruns.Test/compileruns, notably compiling the macros which reference code in thecom.dwolla.config.smithy_shaded.com.amazonaws.kmspackage, which is not yet modified to be package-private.Compile/compilefor semantic information. This is when the package-private modification happens.testis executed, sbt and/or the incremental compiler are not smart enough to recognize that the macro code needs to be recompiled due to the modifications in step 6, and the tests fail.Prior to this PR, the same would be true if you ran
clean; ++ 2.13; Test/compile; test. After this PR is merged, those commands would succeed, because the Scala 2.13 tests no longer use macros to compile the strings, but instead use the reflection library.Unfortunately, as far as I can tell, the reflection library no longer supports this functionality in Scala 3, where you're expected to use macros instead. Luckily we don't really need to run
Test/compilein our builds and can just usecompileinstead.MiMa Exclusions
Updating Smithy4s to
v0.18.25to pull the fixes to codegen caching also includes a welcome change to make the library friendlier for use with cats-tagless. The changes to the generated code are not binary compatible, but as far as I can tell the problematic methods are not accessible outside thecom.dwolla.config.smithy_shaded.com.amazonaws.kmspackage, so we make no binary compatibility guarantees to downstream users who use them directly anyway.