search

trinodb / trino

Official repository of Trino, the distributed SQL query engine for big data, formerly known as PrestoSQL (https://trino.io)
https://trino.io
Apache License 2.0
10.26k stars 2.95k forks source link

Migrate unit tests to JUnit 5 #9378

Open martint opened 3 years ago

martint commented 3 years ago
Damans227 commented 3 years ago

@martint I would like to be part of this project and can help you in planning and executing the roadmap for it. I am willing to dedicate a fixed amount of hours on daily basis for next few months if that helps. Lemme know.

martint commented 3 years ago

@Damans227, that would be great! The changes can be done mostly incrementally by tackling individual test classes. Take a look at the PR I linked above for an example.

Damans227 commented 3 years ago

@martint Ok, scanning through the files changed in this PR: #9362, it appears to me that we are updating the tests from using org.testng framework to Junit5. So, which modules in the codebase do we aim to cover as part of this issue/ project ? Since, you've already updated trino-main & trino-parser in core, should I continue with other modules in the same folder ?

martint commented 3 years ago

I only updated trino-parser and a small portion of trino-main. The goal would be to replace it everywhere.

Damans227 commented 3 years ago

Got it! Thanks for replying.

Damans227 commented 3 years ago

I have updated the tests in the sql>analyzer module. How do I validate my changes before sending a PR ? Is there a maven command to do so ? Apologies in advance as these are very rookie questions.

martint commented 2 years ago
  • provide a checker ensuring that a JUnit test does not extend from TestNG-annotated test class, to avoid situation when @Test or setup methods are inherited but not firing up
  • provide a checker ensuring that a TestNG test does not extend from JUnit-annotated test class, to avoid situation when @Test or setup methods are inherited but not firing up
  • provide a checker ensuring a TestNG test doesn't use JUnit setup/cleanup annotations (eg cleanup might not get fired)

These are handled indirectly by the ReportUnannotatedMethods check

findepi commented 2 years ago

These are handled indirectly by the ReportUnannotatedMethods check

In one direction ("checker ensuring that a TestNG test does not extend from JUnit-annotated test class"), yes.

martint commented 2 years ago

It works in both directions. The checker identifies the junit test methods as unannotated (with TestNG annotations), regardless of whether they are in the parent or child class.

findepi commented 2 years ago

The checker won't run for a child class.

martint commented 2 years ago

The checker won't run for a child class.

I tried it, it works.