Skip to content

Conversation

@len548
Copy link
Contributor

@len548 len548 commented Feb 11, 2026

What changes were proposed in this pull request?

As a sub-task of HDDS-12355, I converted OzoneFileSystemTests to abstract class (previously it was static final) and renamed it to OzoneFileSystemTestBase. This enables template method pattern and abstracts similar tests between AbstractOzoneFileSystemTest and AbstractRootedOzoneFileSystemTest easier.
This PR also addresses:

  • Remove testOzoneFsServiceLoader from both test classes because FileSystem.getFileSystemClass() is Hadoop's code, not Ozone's. We should test our implementation, not the framework's behavior
  • Abstract testCreateKeyWithECReplicationConfig and testListStatusIteratorWithDir to OzoneFileSystemTestBase class.
  • Move setPageSize method to a new class OzoneFileSystemTestUtils from old OzoneFileSystemTests.

Since there are a lot of duplicates in the two test classes, subsequent Jira tickets will continue to evaluate other duplicate tests if this approach is accepted.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-14608

How was this patch tested?

Existing test suites are not broken by this PR.
CI pass: https://github.com/len548/ozone/actions/runs/21897543317

@len548
Copy link
Contributor Author

len548 commented Feb 11, 2026

@adoroszlai can you review please when you have time?

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @len548 for the patch.

Comment on lines +48 to 50
protected OzoneFileSystemTestBase() {
// no instances
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment is no longer valid. The constructor can be completely removed, we can rely on the default one.

Comment on lines -70 to +56
public static void listStatusIteratorOnPageSize(OzoneConfiguration conf,
protected void listStatusIteratorOnPageSize(OzoneConfiguration conf,
Copy link
Contributor

Choose a reason for hiding this comment

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

This and other existing methods that are not going to be overridden can still be static.

Comment on lines +968 to +969
RemoteIterator<FileStatus> listStatusIterator(Path path) throws IOException {
return o3fs.listStatusIterator(path);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's define public abstract FileSystem getFs() in the parent class (implementations already exist, just add @Override). This way we don't need to add a template method for each FileSystem operation (like listStatusIterator(Path)) in the tests.

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.

2 participants