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

Use temporary folder for tests #71

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chris-allan
Copy link

@chris-allan chris-allan commented Jul 31, 2020

Slightly refactored version of the temporary folder test changes originally proposed in #70.

Test cases now use a temporary folder and subclasses are responsible for setting up the n5 implementation.

@chris-allan chris-allan mentioned this pull request Jul 31, 2020
@chris-allan
Copy link
Author

chris-allan commented Jul 31, 2020

The original implementation in #70 had the temporary folder rule in the N5FSTest derived class. Since the AbstractN5Test base abstract class will be used by other filesystem based test harnesses I made the decision to place the JUnit resource management rule there. An argument could certainly be made that it should be the responsibility of the derived class to handle such resource management as in the case of object storage based test harnesses it will not be used.

Happy to go with either option.

Edit: The original abstract method style could also be restored with something to the effect of:

protected abstract N5Writer createN5Writer() throws IOException;

@Before
public void setUp() throws IOException {

  n5 = createN5Writer();
}

@chris-allan
Copy link
Author

@axtimwalde: Was this what you were thinking?

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

Successfully merging this pull request may close these issues.

1 participant