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

Test NG / Spring Boot #44

Closed
jeanbisutti opened this issue Jan 12, 2020 · 15 comments
Closed

Test NG / Spring Boot #44

jeanbisutti opened this issue Jan 12, 2020 · 15 comments

Comments

@jeanbisutti
Copy link
Collaborator

A QuickPerf Test NG listener is developed (see #32 and 3932345 ).

A Spring Boot application is tested with QuickPerf+JUnit4 and QuickPerf+Junit5.

The aim of this issue is to provide a Github repository containing this Spring Boot application with tests using QuickPerf and Test NG.

@jeanbisutti jeanbisutti self-assigned this Jan 19, 2020
@jeanbisutti jeanbisutti changed the title Test NG / Spring Boot example Test NG / Spring Boot Feb 2, 2020
@jeanbisutti jeanbisutti removed their assignment Apr 28, 2020
@hedleyproctor
Copy link

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Contributor

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
Copy link
Collaborator Author

Thank you @oliver-hughes!

I see what happens from your repo.

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.

@hedleyproctor
Copy link

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
Copy link

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
Copy link
Collaborator Author

jeanbisutti commented 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 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?

I don't know. QuickPerfSpringTestNGListener is not very long?

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.

@oliver-hughes
Copy link
Contributor

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
Copy link
Collaborator Author

Hi @oliver-hughes and @hedleyproctor,

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?

@oliver-hughes
Copy link
Contributor

Hi @jeanbisutti, sure - I'd be happy to create a pull request with this.
A couple of points of query first:

  1. 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.
  2. 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?

@jeanbisutti
Copy link
Collaborator Author

Hi @oliver-hughes,

Great!

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.

@hedleyproctor
Copy link

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
Copy link
Collaborator Author

jeanbisutti commented 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
Copy link
Collaborator Author

Fixed with 7b9487a

Thanl you @oliver-hughes and @hedleyproctor!!

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

No branches or pull requests

3 participants