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

Add a README.md to test/BaselineOutput #100

Closed
markusweimer opened this issue May 9, 2018 · 8 comments
Closed

Add a README.md to test/BaselineOutput #100

markusweimer opened this issue May 9, 2018 · 8 comments
Labels
nit Needs a really small fix up-for-grabs A good issue to fix if you are trying to contribute to the project

Comments

@markusweimer
Copy link
Member

markusweimer commented May 9, 2018

We should have a README.md to the test/BaselineOutput folder to explain its purpose.

(Updated on 2018-07-02 to reflect the new location of the folder)

@glebuk
Copy link
Contributor

glebuk commented May 9, 2018

We should also consider moving the folder under /tests to make the purpose more obvious.

@Ivanidzo4ka
Copy link
Contributor

Does anyone remember why it's called ZBaselines instead of just baselines or TestBaselines?

@costin-eseanu
Copy link

Because it would be sorted last in the folder view, so it's easy to find. :)

@justinormont
Copy link
Contributor

Agreed w / @glebuk & @Ivanidzo4ka (move+rename). Placing under /test/baselineOutput/ may be easier to understand its purpose.

@Ivanidzo4ka
Copy link
Contributor

So if we call it Baselines inside test folder, it would be first, and it would be easy to find as well!
Should we create issue to move baselines to test folder and rename them?

@justinormont
Copy link
Contributor

What do we think the best name for it is? Terse + understandable is my metric to optimize.

Also, welcome @costin-eseanu!

@TomFinley
Copy link
Contributor

TomFinley commented May 10, 2018

@justinormont I like your /test/BaselineOutput suggestion.

The intention of that directory is that it mirrors the TestOutput directory, produced by our tests. (In this way we can review any divergences between the expected output (in the baselines), and the actual output (as actually output by the test.

This however brings up what might be a larger (different?) issue. The test outputs used to be written just to the root directory, alongside ZBaselines. I might like them to be written to /test/TestOutput... that way, you get the two peer directories right next to each other.

Right now, as near as I can tell, the test output is not being written to a single directory, which makes tracking changes to baselines harder to find, and makes the practice of using diff tools to see all the test divergences at a glance now impossible (I have, for example, one TestOutput directory at Microsoft.ML.Core.Tests\netcoreapp2.0\TestOutput in my build, and another TestOutput directory at ``). Also it's being put in a very, very odd location: the bin directory right next to the test assemblies. I'm not sure the binary directory is really an appropriate location for test output.

I'm not sure why it was changed to this new system exactly, but we should revisit, maybe instead writing them to test/TestOutput, or something.

Also hi @costin-eseanu , you seem familiar somehow. :D

@markusweimer markusweimer changed the title Add a README.md to ZBaselines Add a README.md to test/BaselineOutput Jul 2, 2018
@Ivanidzo4ka Ivanidzo4ka added the up-for-grabs A good issue to fix if you are trying to contribute to the project label Oct 18, 2018
@Ivanidzo4ka
Copy link
Contributor

DRI RESPONSE: yes, we probably should. Basically this is folder where we keep baseline files for our tests.
Marking this as up-for-grabs.

@Ivanidzo4ka Ivanidzo4ka added the nit Needs a really small fix label Oct 18, 2018
@shauheen shauheen closed this as completed Dec 6, 2018
@ghost ghost locked as resolved and limited conversation to collaborators Mar 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
nit Needs a really small fix up-for-grabs A good issue to fix if you are trying to contribute to the project
Projects
None yet
Development

No branches or pull requests

7 participants