unittest vs pytest (again)

keunwoochoi / torchaudio-contrib

A test bed for updates and new features | pytorch/audio

169 stars 22 forks source link

unittest vs pytest (again) #39

Open keunwoochoi opened 5 years ago

keunwoochoi commented 5 years ago

We once talked about it briefly on #25. So, we have some preferences on pytest over unittest. But currently both pytorch/pytorch and pytorch/audio are using unittest. I think we should follow them, otherwise - if we go with pytest - we'd need to write it again with unittest. Any thought?

hagenw commented 5 years ago

I'm not completely convinced. Maybe we should first:

Ask PyTorch if they will stay with unittest
Consider that unittest might also be run by pytest. Haven't tested this, but https://docs.pytest.org/en/latest/unittest.html

The main reason I'm still pushing for pytest is that it is very convenient for testing a huge combination of parameters/arguments and in my opinion the better choice.

keunwoochoi commented 5 years ago

Hey, thanks for the opinion. I totally got it. But Pytorch is huge, it'd take a good time to persuade them and make the change while we need to decide it much sooner. But maybe torchaudio would be willing to change it and that's what really matters to us.

So, how about asking torchaudio if they'd be fine with pytest? Actually, could you do that? :) that'll be more persuasive since you know better.

hagenw commented 5 years ago

Regarding PyTorch there is an ongoing discussion in https://github.com/pytorch/pytorch/issues/11578, but I think we don't have to make our decision based on the outcome. Both pytorch/vision and pytorch/audio are self-contained repositories with there own testing framework.

So, indeed, we should ask pytorch/audio if they are willing to include pytest as a dependency in there tests. I will make a proposal and ask them.

hagenw commented 5 years ago

I asked them (https://github.com/pytorch/audio/issues/102) and added a pull request for automatic testing on travis (https://github.com/pytorch/audio/pull/100)

keunwoochoi commented 5 years ago

Perfect, thanks a lot. To not block #34, I'll merge it with unittest at the moment. @ksanjeevan agreed that he's willing to change it to pytest later.

hagenw commented 5 years ago

Don't worry, we don't have to change existing unittest as they will be executed by pytest as well. If we add further tests or can improve existing ones with pytest then it is worth considering a rewrite.

keunwoochoi commented 5 years ago

@hagenw Perfecter!

keunwoochoi commented 5 years ago

https://github.com/pytorch/audio/issues/102#issuecomment-489332202

Ok, let's go with pytest @hagenw Could you possibly make a PR for it?

hagenw commented 5 years ago

Sure, will do until end of week.

On 12 May 2019 at 23:01, wrote:

pytorch/audio#102 (comment)

Ok, let's go with pytest @hagenw Could you possibly make a PR for it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.