search

PyWorkflowApp / visual-programming

A Python Visual Programming Workspace for Data Science
MIT License
31 stars 12 forks source link

Increases code coverage of UI #88

Open cesaragv opened 4 years ago

cesaragv commented 4 years ago

This PR pushes the code coverage above 64%

reelmatt commented 4 years ago

Looking good! I see you also fixed some of the hook errors too, so close to passing!!

reddigari commented 4 years ago

Amazing job on the tests!!

I need to say though that switching from hooks to class methods quickly, just for the sake of testing, is dangerous. The reason they introduced hooks was to remove a lot of the bugginess and verbosity of class-based callbacks (definint state in constructors, binding every method, etc.). My fear is that changing this much at the last second will result in us releasing something that doesn't work.

That being said, the things I tried worked fine. Just a little hesitant. Thoughts?

cesaragv commented 4 years ago

As @reddigari even though I made the changes, I am also hesitant. In the end, I found a way to make hook works with enzyme and mount, but would drastically reduce the code coverage (maybe just above 60%). I proposed to hold on on this PR, and mention the code coverage and testing results, but merge after the presentation on Thursday. If anything breaks, we would calmy have time to fix it. Otherwise, we might rush fixes that could potentially break other things

cesaragv commented 4 years ago

Unless, of course, somebody has time to do a full manual regression