I think one thing that might help guide this discussion and make reviewing your pull request easier is for you to write up a YTEP document that goes a little bit into the technical and social details behind this change. Things that would be good to hear about:
* What do future contributors need to do to add answer tests in the new framework? * What are differences between nose and pytest and developers need to be aware of? * Are there any downsides to making this change? * Any technical details that might be helpful for reviewers of your pull request to read in a more narrative fashion than is possible in a pull request.
Writing up a YTEP is a great way to both document the change in a format that will be preserved long-term and a way for you to explain what's happening without others needing to read over all the code changes. It's also a good thing for your personal resume to be able to point to a design document for a major development push along with the code and pull request(s).
Separately from the YTEP, I'd like to encourage you to think about breaking up your pull request into multiple chunks that can be upstreamed one at a time. This might mean running both pytest and nose for a while as things get converted. I could imagine this going like, first pull request sets up the pytest infrastructure and gets pytest running but with no actual tests running under pytest. Second pull request converts some subsection of the code to run under pytest. Third pull request does another subsection and so on until the entire codebase is converted. That will probably make it much easier for reviewers to understand your changes and make it easier for reviewers to spot bugs and possible issues.
I understand the desire to upstream something like this all at once but I think there's a big concern about making big changes to the codebase that are difficult to review all at once.
Hope that's helpful, please feel free to ping me on github if you want my help with anything.
On Mon, Sep 30, 2019 at 9:29 AM Matthew Turk firstname.lastname@example.org wrote:
Thanks for doing all of this.
One of the big things in this, that I think everybody should know about, is that it also changes our answer testing to use flat hashes of array values, instead of *full* array values that are stored in some remote location. This means that the results of the answer tests are co-located with the code, and much easier for folks to set up and use.
On Sat, Sep 28, 2019 at 10:31 AM Coughlin, Jared W email@example.com wrote:
I've been working on a PR (https://github.com/yt-project/yt/pull/2286)
that converts yt's testing framework from nose to pytest. I believe that it's mostly ready to go, so I was writing to see what everyone's thoughts on this change are, if there is any feedback on my implementation, and if anyone would be willing to help review it, since there are quite a few changes. Thanks, and have a good weekend!
-Jared _______________________________________________ yt-dev mailing list -- firstname.lastname@example.org To unsubscribe send an email to email@example.com
yt-dev mailing list -- firstname.lastname@example.org To unsubscribe send an email to email@example.com