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

[WFLY-17740] Add missing @fallback test in Microprofile Fault Tolerance quickstart #656

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

ChristinaDsl
Copy link
Contributor

@ChristinaDsl ChristinaDsl commented Mar 17, 2023

https://issues.redhat.com/browse/WFLY-17740
In this issue were added two Test methods, both testing a method with @fallback annotation . The first one tests the successful execution of the method, which also has specific time frame (with the help of @timeout). The second one secures that every time the execution time of the method exceeds the time limit, the Fallback method executed successfully.

@iweiss
Copy link
Contributor

iweiss commented Mar 20, 2023

Please add the link to the issue tracker on the first line

@iweiss
Copy link
Contributor

iweiss commented Jun 14, 2023

@emmartins can you review this, please?

@emmartins
Copy link
Contributor

@pferraro @rhusar not sure who is now the responsible for this QS, my guess it's one of you, please review

@rhusar
Copy link
Member

rhusar commented Jun 15, 2023

@pferraro @rhusar not sure who is now the responsible for this QS, my guess it's one of you, please review

@emmartins That would be me (btw a good hint is available in Maven project model - https://github.com/wildfly/quickstart/blob/main/microprofile-fault-tolerance/pom.xml#L40-L44)

@rhusar
Copy link
Member

rhusar commented Jun 15, 2023

@emmartins I can't assign myself as reviewer - can you please fix the project permissions settings? Thanks!

@ChristinaDsl ChristinaDsl force-pushed the WFLY-17740 branch 2 times, most recently from ae11320 to 3fc846a Compare June 20, 2023 11:32
@ChristinaDsl
Copy link
Contributor Author

@rhusar Thank you. All changes have been made.

@ChristinaDsl ChristinaDsl requested a review from rhusar July 13, 2023 09:51
@ChristinaDsl
Copy link
Contributor Author

@rhusar Is there anything else that you need from my side?

@rhusar
Copy link
Member

rhusar commented Aug 2, 2023

@rhusar Is there anything else that you need from my side?

Nope - just competing priorities on my side to get to do a full review...

Copy link
Contributor

@emmartins emmartins left a comment

Choose a reason for hiding this comment

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

The quickstart testing has been fully reworked, please rebase and update changes.

Copy link
Member

@rhusar rhusar left a comment

Choose a reason for hiding this comment

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

@ChristinaDsl Please review the prior changes to the testing and note that we have removed ARQ and reworked to use a remote client istead.

microprofile-fault-tolerance/pom.xml Outdated Show resolved Hide resolved
microprofile-fault-tolerance/pom.xml Outdated Show resolved Hide resolved
Comment on lines 52 to 59
@ArquillianResource
private URL deploymentUrl;

@Inject
private CoffeeResource coffeeResource;

@Inject
CoffeeRepositoryService coffeeRepository;
Copy link
Member

Choose a reason for hiding this comment

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

Remove this as this won't work when testing against deployment running in e.g. OpenShift.

@ChristinaDsl
Copy link
Contributor Author

@rhusar thanks for your comments. I will manage the fixes.

@emmartins
Copy link
Contributor

@ChristinaDsl please verify the new test, one run on CI did fail

@rhusar
Copy link
Member

rhusar commented Jan 19, 2024

@emmartins Yet again - could you please verify the permissions for this repository? I can't even mark (my own) conversations as resolved...

@emmartins
Copy link
Contributor

@rhusar if I find that option... yes sure!!!!

@rhusar
Copy link
Member

rhusar commented Jan 19, 2024

@rhusar if I find that option... yes sure!!!!

I am going to guess that it's just the triage permission.

@emmartins
Copy link
Contributor

emmartins commented Jan 19, 2024

@rhusar can you please recheck now?

@ChristinaDsl
Copy link
Contributor Author

@emmartins Sure.

Copy link
Contributor

@emmartins emmartins left a comment

Choose a reason for hiding this comment

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

The tests continue to produce intermittent results, for instance:

Error: Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 1.893 s <<< FAILURE! -- in org.wildfly.quickstarts.microprofile.faulttolerance.CoffeeResourceIT
Error: org.wildfly.quickstarts.microprofile.faulttolerance.CoffeeResourceIT.testCoffeeRecommendations -- Time elapsed: 0.282 s <<< FAILURE!
java.lang.AssertionError: expected:<2> but was:<1>

@emmartins
Copy link
Contributor

Merging since testsuite seems stable now, if an intermitent pops up again will open an issue, thanks @ChristinaDsl

@emmartins emmartins merged commit 016c20b into wildfly:main Apr 3, 2024
21 checks passed
@rhusar
Copy link
Member

rhusar commented Apr 3, 2024

Thanks @ChristinaDsl and @emmartins !

@ChristinaDsl
Copy link
Contributor Author

Thank you too @emmartins and @rhusar!

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.

4 participants