Skip to content

Conversation

@devmadhuu
Copy link
Contributor

What changes were proposed in this pull request?

This PR is raised to incorporate below tasks:

AbstractReconSqlDBTest uses Files.createTempDirectory. We should replace it with @TempDir. The problem is that AbstractReconSqlDBTest is used in two different ways:

Several tests extend AbstractReconSqlDBTest. JUnit lifecycle method is executed, @TempDir can be used.
ReconTestInjector and TestReconWithDifferentSqlDBs create instance of AbstractReconSqlDBTest explicitly. These should provide temp directory via parameter.

What is the link to the Apache JIRA

Please create an issue in ASF JIRA before opening a pull request, and you need to set the title of the pull
request which starts with the corresponding JIRA issue number. (e.g. HDDS-XXXX. Fix a typo in YYY.)

(Please replace this section with the link to the Apache JIRA)
https://issues.apache.org/jira/browse/HDDS-9968

How was this patch tested?

This patch is being tested by executing all existing test cases which are extending and using AbstractReconSqlDBTest and ReconTestInjector classes.

@devmadhuu devmadhuu marked this pull request as draft January 19, 2024 11:53
@devmadhuu devmadhuu marked this pull request as ready for review January 19, 2024 14:48
@devmadhuu
Copy link
Contributor Author

@adoroszlai
Copy link
Contributor

@ivandika3 would you like to review?

@adoroszlai adoroszlai changed the title HDDS-9968. Avoid using Files.createTempDirectory in AbstractReconSqlDBTest. HDDS-9968. Avoid using Files.createTempDirectory in AbstractReconSqlDBTest Jan 19, 2024
@ivandika3
Copy link
Contributor

Thanks @devmadhuu for the patch. The changes make sense. LGTM +1.

Learnt that if a test is constructing the AbstractReconSqlDBTest directly, the test is responsible to create the schema since it will not trigger BeforeEach create schema hook.

@adoroszlai adoroszlai merged commit 95d9222 into apache:master Jan 20, 2024
@adoroszlai
Copy link
Contributor

Thanks @devmadhuu for the patch, @ivandika3 for the review.

Tejaskriya pushed a commit to Tejaskriya/ozone that referenced this pull request Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants