You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Can you give any more information about what would be required to get QuickPerf working correctly with Spring? Specifically if we are using integration tests that extend the AbstractTransactionalTestNGSpringContextTests class. Is there a problem with the TestNG listener, or is it more to do with how the datasource / session factory / transactions are working? We have started trying to implement QuickPerf and have added a proxy to the datasource. If there are code changes required so that the QuickPerf annotations will be read then I may be able to have a look at them if you can give me more information. Thanks.
I have never tested QuickPerf + Test NG + Spring Boot. So, I am not sure that the SQL annotations would work with this configuration. It may have issues with AbstractTransactionalTestNGSpringContextTests. Could you share an example with AbstractTransactionalTestNGSpringContextTests and the proxy on the datasource? So, I could also look at this example, and help have something working.
One issue may be that the performance recording does not start just before the test method execution and does not stop just after the test method execution (code). It is difficult to say without an example.
I've written a small example to show the annotations not working with Spring: repo.
As far as I can tell none of the QuickPerf annotations will cause the Spring Test to fail.
QuickPerfTestNGListener is not called during the execution. The reason may be that AbstractTestNGSpringContextTests and QuickPerfTestNGListener both implement IHookable. QuickPerfTestNGListener implements IHookable to remove an allocation offset related to the framework test code execution for some JVM annotations. I identified a potential solution to have the QuickPerf SQL annotations work with Test NG + Spring. I started the beginning of this possible solution here. The Spring test class could extend this one. It remains to report some pieces of code (what is not related to the JVM annotations) from QuickPerfTestNGListener to QuickPerfTestExecutionListener, and check that then the SQL annotations work.
Thanks for doing this. I think myself and Oliver understand the problem better now. We agree that because the Spring base class implements IHookable this is conflicting with the listener. You know this code better than us, are you happy to try and finish off the implementation? Or if not let us know and we can try.
So are you planning on having two methods of running TestNG tests? One for non-Spring code and one for using Spring? If so will you move the code that needs to be run before and after tests to a central class and then invoke that from both QuickPerfTestNGListener and the Spring listener? I would be tempted to rename the second listener to QuickPerfSpringTestExecutionListener to make it clearer that it is specific to Spring.
I have good news. I have found a solution to have QuickPerf SQL annotations work with Test NG + Spring. The code is here. The solution is based on an IInvokedMethodListener implementation. With the Spring TestExecutionListener it seems difficult to notify TestNG of a test failure simply. To use the IInvokedMethodListener, you need to add two files to your project: this one and this one. You can remove the quick-perf-testng dependency.
So are you planning on having two methods of running TestNG tests? One for non-Spring code and one for using Spring?
Yes, I would prefer today. The reason is it is far more challenging to run the JVM annotations with Spring. The QuickPerfSpringRunner manages both SQL and JVM annotations for Spring + JUnit 4. The implementation is rather complex. To execute the SQL annotations with Test NG + Spring could already help QuickPerf users.
QuickPerfSpringTestExecutionListener to make it clearer that it is specific to Spring.
Good point. QuickPerfSpringTestNGListener is the name the implementation of IInvokedMethodListener `
If so will you move the code that needs to be run before and after tests to a central class and then invoke that from both QuickPerfTestNGListener and the Spring listener?
Could you please use the QuickPerfSpringTestNGListener on your Test NG projects and check that the SQL annotations work as expected, especially with tests using transactions? Also, could you please create a reproducing example in case of unexpected behavior?
Once the implementation seems stable, we could provide it with a QuickPerf dependency to help other QuickPerf users.
Hi @jeanbisutti,
Thanks for the quick help! So far the solution seems to be working in another TestNG & Spring project, everything is working as expected with the SQL annotations.
So far the solution seems to be working in another TestNG & Spring project, everything is working as expected with the SQL annotations.
@oliver-hughes Thank you very much for having tested with another TestNG & Spring project.
Now we could add the new code to QuickPerf. A new quick-perf-spring-testng artifact could contain the QuickPerfSpringTestNGListener and the org.testng.ITestNGListener file. Automatic non-regression tests should also be added, an example. @oliver-hughes and @hedleyproctor would you be interested in creating a QuickPerf pull request with my help, or would you prefer that I do it?
Hi @jeanbisutti, sure - I'd be happy to create a pull request with this.
A couple of points of query first:
I don't believe the org.testng.ITestNGListener file should be included? There are a few different ways the listener could be used with TestNG described here, so I think it would be best to leave this to the user.
I also think that the implementation is not specific to spring, but rather could work with any TestNG project? So perhaps the naming of the artifact/listener should reflect this?
I don't believe the org.testng.ITestNGListener file should be included? There are a few different ways the listener could be used with TestNG described here, so I think it would be best to leave this to the user.
Yes, good point!
I also think that the implementation is not specific to spring, but rather could work with any TestNG project? So perhaps the naming of the artifact/listener should reflect this?
I agree. Something like QuickPerfSqlTestNGListener?
Don't hesitate to ask if you have other remarks or need help.
Myself and Oliver have tried to build the QuickPerf master branch but it is failing to get a dependency:
Failed to execute goal on project quick-perf-jfr-annotations: Could not resolve dependencies for project org.quickperf:quick-perf-jfr-annotations:jar:1.1.1-SNAPSHOT: Failure to find org.openjdk.jmc:flightrecorder.rules.jdk:jar:7.1.1
Looking on Maven Central I can see only Flight Recorder v8.
We could try to clone and build the Flight Recorder project locally but is this the best solution? Or alternatively could we upgrade QuickPerf to use Flight Recorder v8? We could do an issue and PR to upgrade the library version if you think the upgrade would be easy?
Activity
[-]Test NG / Spring Boot example[/-][+]Test NG / Spring Boot[/+]hedleyproctor commentedon Feb 8, 2022
Can you give any more information about what would be required to get QuickPerf working correctly with Spring? Specifically if we are using integration tests that extend the AbstractTransactionalTestNGSpringContextTests class. Is there a problem with the TestNG listener, or is it more to do with how the datasource / session factory / transactions are working? We have started trying to implement QuickPerf and have added a proxy to the datasource. If there are code changes required so that the QuickPerf annotations will be read then I may be able to have a look at them if you can give me more information. Thanks.
jeanbisutti commentedon Feb 8, 2022
I have never tested QuickPerf + Test NG + Spring Boot. So, I am not sure that the SQL annotations would work with this configuration. It may have issues with AbstractTransactionalTestNGSpringContextTests. Could you share an example with AbstractTransactionalTestNGSpringContextTests and the proxy on the datasource? So, I could also look at this example, and help have something working.
jeanbisutti commentedon Feb 8, 2022
One issue may be that the performance recording does not start just before the test method execution and does not stop just after the test method execution (code). It is difficult to say without an example.
oliver-hughes commentedon Feb 9, 2022
I've written a small example to show the annotations not working with Spring: repo.
As far as I can tell none of the QuickPerf annotations will cause the Spring Test to fail.
jeanbisutti commentedon Feb 9, 2022
Thank you @oliver-hughes!
I see what happens from your repo.
QuickPerfTestNGListener is not called during the execution. The reason may be that
AbstractTestNGSpringContextTestsandQuickPerfTestNGListenerboth implementIHookable.QuickPerfTestNGListenerimplementsIHookableto remove an allocation offset related to the framework test code execution for some JVM annotations. I identified a potential solution to have the QuickPerf SQL annotations work with Test NG + Spring. I started the beginning of this possible solution here. The Spring test class could extend this one. It remains to report some pieces of code (what is not related to the JVM annotations) from QuickPerfTestNGListener to QuickPerfTestExecutionListener, and check that then the SQL annotations work.hedleyproctor commentedon Feb 10, 2022
Thanks for doing this. I think myself and Oliver understand the problem better now. We agree that because the Spring base class implements IHookable this is conflicting with the listener. You know this code better than us, are you happy to try and finish off the implementation? Or if not let us know and we can try.
hedleyproctor commentedon Feb 10, 2022
So are you planning on having two methods of running TestNG tests? One for non-Spring code and one for using Spring? If so will you move the code that needs to be run before and after tests to a central class and then invoke that from both QuickPerfTestNGListener and the Spring listener? I would be tempted to rename the second listener to QuickPerfSpringTestExecutionListener to make it clearer that it is specific to Spring.
jeanbisutti commentedon Feb 10, 2022
Hi @hedleyproctor and @oliver-hughes ,
I have good news. I have found a solution to have QuickPerf SQL annotations work with Test NG + Spring. The code is here. The solution is based on an
IInvokedMethodListenerimplementation. With the SpringTestExecutionListenerit seems difficult to notify TestNG of a test failure simply. To use theIInvokedMethodListener, you need to add two files to your project: this one and this one. You can remove thequick-perf-testngdependency.Yes, I would prefer today. The reason is it is far more challenging to run the JVM annotations with Spring. The
QuickPerfSpringRunnermanages both SQL and JVM annotations for Spring + JUnit 4. The implementation is rather complex. To execute the SQL annotations with Test NG + Spring could already help QuickPerf users.QuickPerfSpringTestExecutionListener to make it clearer that it is specific to Spring.Good point.
QuickPerfSpringTestNGListener is the name the implementation ofIInvokedMethodListener `If so will you move the code that needs to be run before and after tests to a central class and then invoke that from both QuickPerfTestNGListener and the Spring listener?I don't know. QuickPerfSpringTestNGListener is not very long?
Could you please use the
QuickPerfSpringTestNGListeneron your Test NG projects and check that the SQL annotations work as expected, especially with tests using transactions? Also, could you please create a reproducing example in case of unexpected behavior?Once the implementation seems stable, we could provide it with a QuickPerf dependency to help other QuickPerf users.
oliver-hughes commentedon Feb 11, 2022
Hi @jeanbisutti,
Thanks for the quick help! So far the solution seems to be working in another TestNG & Spring project, everything is working as expected with the SQL annotations.
jeanbisutti commentedon Feb 11, 2022
Hi @oliver-hughes and @hedleyproctor,
@oliver-hughes Thank you very much for having tested with another TestNG & Spring project.
Now we could add the new code to QuickPerf. A new
quick-perf-spring-testngartifact could contain theQuickPerfSpringTestNGListenerand theorg.testng.ITestNGListenerfile. Automatic non-regression tests should also be added, an example. @oliver-hughes and @hedleyproctor would you be interested in creating a QuickPerf pull request with my help, or would you prefer that I do it?oliver-hughes commentedon Feb 16, 2022
Hi @jeanbisutti, sure - I'd be happy to create a pull request with this.
A couple of points of query first:
org.testng.ITestNGListenerfile should be included? There are a few different ways the listener could be used with TestNG described here, so I think it would be best to leave this to the user.jeanbisutti commentedon Feb 17, 2022
Hi @oliver-hughes,
Great!
Yes, good point!
I agree. Something like
QuickPerfSqlTestNGListener?Don't hesitate to ask if you have other remarks or need help.
hedleyproctor commentedon Feb 21, 2022
Myself and Oliver have tried to build the QuickPerf master branch but it is failing to get a dependency:
Failed to execute goal on project quick-perf-jfr-annotations: Could not resolve dependencies for project org.quickperf:quick-perf-jfr-annotations:jar:1.1.1-SNAPSHOT: Failure to find org.openjdk.jmc:flightrecorder.rules.jdk:jar:7.1.1
Looking on Maven Central I can see only Flight Recorder v8.
We could try to clone and build the Flight Recorder project locally but is this the best solution? Or alternatively could we upgrade QuickPerf to use Flight Recorder v8? We could do an issue and PR to upgrade the library version if you think the upgrade would be easy?
jeanbisutti commentedon Feb 21, 2022
The JFrog repository provides the missing dependency (QuickPerf pom)
I have just removed the 7.1.1 JMC dependencies from my local repository. After that, I can see that Maven can download them.
I suspect that Maven only uses the Maven central repository and blocks the others in your environment.
v8 may have an impact, for example on @ExpectNoJvmIssue.
Could you try downloading the 7.1.1 dependencies from the JFrog repository or building from v8 and remove v8 for your PR?
jeanbisutti commentedon Mar 3, 2022
Fixed with 7b9487a
Thanl you @oliver-hughes and @hedleyproctor!!