-
Notifications
You must be signed in to change notification settings - Fork 102
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
Vulcan testkit #629
Vulcan testkit #629
Conversation
Is it possible to re-run CI? I initially targetted the wrong branch which is the cause of the test failures |
ebbe4da
to
780423b
Compare
Not sure why this build is failing? |
So far as I can make out the issue is that this is a new module so mima is not able to check binary compat against a previous version, not sure how best to get around this |
The new module needs to be temporarily excluded from Mima. There’s probably a cleaner way to do it but in the meantime I’m happy to do it myself the hacky way. |
clientSettings: SchemaRegistryClientSettings[IO], | ||
name: String = "schema-compatibility-checker" | ||
) = new Fixture[CompatibilityChecker[IO]](name) { | ||
private var checker: CompatibilityChecker[IO] = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this be a lazy val rather than a var initialized in beforeAll
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that might mess with test timings then, as the first test to use the fixture would have the spin-up time bundled in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking into the effect in question, don't think its actually doing any IO, so shouldn't add too much overhead to the first test to use it, that said I think I still find spinning up in the beforeAll slightly neater, even if the presence of null
in the codebase makes me feel slightly ill...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it probably doesn't matter much either way.
Sorry to be so slow with this! I'm going to take the liberty of splitting this into |
On second thought, let's just leave this as one module called vulcan-testkit-munit unless and until we want to add another test framework integration. |
Per discussion on gitter, an munit fixture to facilitate testing avro schema compatibility against the schema registry. Feedback welcome :)