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

move TestHelpers.OAs -> Test.OAs #25568

Closed
wants to merge 1 commit into from
Closed

move TestHelpers.OAs -> Test.OAs #25568

wants to merge 1 commit into from

Conversation

rfourquet
Copy link
Member

This is motivated by the fact that I didn't find a way to use the OAs module in Random tests (in #24874, where I had to duplicate temporarily OAs), and that after all this OAs module could very well be useful beyond base/stdlib (i.e. in external packages).
I'm not sure why the module name is not OffsetArrays - maybe to not be confused with the "real" module which is in a package? (cc @timholy).

@rfourquet rfourquet added the testsystem The unit testing framework and Test stdlib label Jan 15, 2018
@KristofferC
Copy link
Sponsor Member

I don't think this should be in the Test module. It is just a utility thing to help test some Base functionality. If people want to test their stuff with OffsetArrays in their packages, they can just use the OffsetArrays package?

@StefanKarpinski
Copy link
Sponsor Member

Agree, this seems strange. Keep in mind that Test is a formal interface that we support. Having random (forgive the pun) stuff in it just for Base's own testing is not ideal.

@rfourquet
Copy link
Member Author

Ok, I agree, and I also remember now that a package can depend on another one only for tests, which makes this dependency optional for using the package (so depending on OffsetArrays for testing is not a big deal).

@rfourquet rfourquet closed this Jan 17, 2018
@rfourquet
Copy link
Member Author

For reference, another (better) fix at #25582.

@rfourquet rfourquet deleted the rf/test-OAs branch January 17, 2018 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testsystem The unit testing framework and Test stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants