Issue272 Add TCK for deployment failure when LRA is not combined with either Compensate or AfterLRA

eclipse / microprofile-lra

microprofile-lra

Apache License 2.0

101 stars 30 forks source link

Issue272 Add TCK for deployment failure when LRA is not combined with either Compensate or AfterLRA #273

Closed xstefank closed 4 years ago

xstefank commented 4 years ago

resolves #272

add a new TCK test - TckInvalidSignaturesTests#invalidLRAResourceWithoutCompensateOrAfterLRATest
fix other TCK LRA resources which do not contain any of the Compensate of AfterLRA methods
ignore TckTests#mixedMultiLevelNestedActivity because it collides with the above requirement (will be reported in the new issue - https://github.com/eclipse/microprofile-lra/issues/274)

xstefank commented 4 years ago

@rdebusscher @mmusgrov @ochaloup this is ready for review again.

xstefank commented 4 years ago

@mmusgrov I am going to merge this later today. Do you still have any questions?

xstefank commented 4 years ago

@mmusgrov @tomjenkinson we have 2 approvals on this PR. I am going to merge this later today so this is just a notification if you want to take another look.

tomjenkinson commented 4 years ago

Thanks @xstefank my personal preference would be to remove the code formatting changes to make the change set smaller. For instance this commit has quite a bit of whitespace: https://github.com/eclipse/microprofile-lra/pull/273/commits/0fd620c31584f327f8e8f75b7518c60f03a832ba

Also, I wonder if the commits related to 272 could be squashed?

xstefank commented 4 years ago

Pushed only changes suggested by @tomjenkinson in his last comment. No functional changes.

tomjenkinson commented 4 years ago

Thanks @xstefank! (I think we should consider to update the checkstyle)

xstefank commented 4 years ago

Merging since this was approved 2 times. We can revert if needed.