search

diffpy / diffpy.fourigui

Other
0 stars 7 forks source link

Test for correct applycutoff #63

Closed sbillinge closed 2 weeks ago

sbillinge commented 2 weeks ago

Closes #61

Replaces #62 and #47. Please see there for extensive discussion.

@bobleesj @cadenmyers13 I wrote a test for the applycutoff() method.

this is what I wanted to do so that we can catch the right behavior.

tbh I don't like the fourigui code, it is not modular and well separated, making it hard to understand and hard to test, but I don't want to do a big refactor on this code.

Still, take a look at the test and hopefully you can see this as an example of how to test a function/method like this. I hope it is helpful. The key difference from before is that I hand-wrote an array that I KNOW has the behavior that I want. It is an easily understandable 5x5x5 array that is the smallest array that can have both a qmin and qmax.

I guess that for all tests to pass we may have to do @cadenmyers13 fix of the h5 test data which is wrong because it was generated by the code which was wrong. But this shows how testing can catch that. If you test code using data that was generated using the code, of couse it will pass, and you are not testing anything really......

sbillinge commented 2 weeks ago

ok, that got a bit messy with missing merges, my apologies for the ugly commit history, but I finally go there. I am inclined to merge it in this messy way since I don't really want to redo it on a clean branch for this code.

take a look and let me know if you have any questions.

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.78%. Comparing base (908992d) to head (fae69cf). Report is 10 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #63 +/- ## ========================================== + Coverage 93.49% 93.78% +0.29% ========================================== Files 4 4 Lines 169 177 +8 ========================================== + Hits 158 166 +8 Misses 11 11 ``` | [Files with missing lines](https://app.codecov.io/gh/diffpy/diffpy.fourigui/pull/63?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=diffpy) | Coverage Δ | | |---|---|---| | [tests/integration\_test.py](https://app.codecov.io/gh/diffpy/diffpy.fourigui/pull/63?src=pr&el=tree&filepath=tests%2Fintegration_test.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=diffpy#diff-dGVzdHMvaW50ZWdyYXRpb25fdGVzdC5weQ==) | `98.03% <ø> (-0.58%)` | :arrow_down: | | [tests/test\_fourigui.py](https://app.codecov.io/gh/diffpy/diffpy.fourigui/pull/63?src=pr&el=tree&filepath=tests%2Ftest_fourigui.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=diffpy#diff-dGVzdHMvdGVzdF9mb3VyaWd1aS5weQ==) | `99.07% <100.00%> (ø)` | |
bobleesj commented 2 weeks ago

@sbillinge Thanks for the demonstration - I haven't used mocking before but I see it can be useful for preventing certain functions like plotting. Yay, no more warnings!

sbillinge commented 2 weeks ago

@sbillinge Thanks for the demonstration - I haven't used mocking before but I see it can be useful for preventing certain functions like plotting. Yay, no more warnings!

tbh, when you have to mock stuff like this it probably means the code design is bad and would benefit from a refactor.....but good to have in the back pocket.

It was ok here because the things I suppressed were functions that should be tested separately, so I just wanted to suppress them in the test and only test the desired behavior on the attributes.