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

Update tests to run on Maven 3.9 #17

Merged
merged 1 commit into from
Sep 20, 2023
Merged

Conversation

willmostly
Copy link
Contributor

Tests that passed on 3.6 are failing on 3.9. It looks like this is potentially due to stricter handling of NPEs. These changes define methods in the stub classes to prevent NPEs in the authentication layer.

Copy link
Member

@vishalya vishalya left a comment

Choose a reason for hiding this comment

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

Let's wait for this one, until we get all the tests running.

@mosabua
Copy link
Member

mosabua commented Aug 28, 2023

We should move away from testng to junit and junit jupyter like Trino

@mosabua mosabua requested review from a team and removed request for a team August 31, 2023 00:15
@mosabua
Copy link
Member

mosabua commented Sep 5, 2023

Do we want to merge this and migrate back to Junit later ? Seems like we could just use junit now .. or is there some obstacle @willmostly ?

@willmostly
Copy link
Contributor Author

Updated to junit 5, @vishalya and I are going to look into the test logic to see why the Ldap tests are getting connection timeouts

<artifactId>mockito-all</artifactId>
<version>1.10.19</version>
<artifactId>mockito-core</artifactId>
<version>5.3.1</version>
Copy link
Member

Choose a reason for hiding this comment

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

Wow .. thats a BIG jump

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah - this is the only usage of Mockito, so fortunately there wasn't anything holding us back. It is needed for junit 5 compatibility

@vishalya
Copy link
Member

@willmostly - I have that test working with some minor changes on top of yours, https://github.com/vishalya/trino-gateway/commits/working_test

@willmostly willmostly force-pushed the will/maven39 branch 3 times, most recently from db847c2 to f0d2b3f Compare September 18, 2023 20:26
Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Looks good now. Code style and further migration to junit will be done in follow up PRs.

@mosabua mosabua merged commit 0415f8f into trinodb:main Sep 20, 2023
2 checks passed
@mosabua mosabua mentioned this pull request Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants