Skip to content

feat: add 2 tests based on good/bad plot from upstream issue#2

Merged
haval0 merged 2 commits intoissue/24092from
test-good-bad-plot
Mar 7, 2023
Merged

feat: add 2 tests based on good/bad plot from upstream issue#2
haval0 merged 2 commits intoissue/24092from
test-good-bad-plot

Conversation

@haval0
Copy link
Owner

@haval0 haval0 commented Mar 1, 2023

The test cases are based on the demonstration of the issue in upstream issue matplotlib#24092

The test case for bad plot (test_tick_values_not_empty) should fail until we implement our fix.

The test case for good plot (test_tick_values_correct) should already succeed and continue to succeed after we implement our fix.

The test cases are based on the demonstration of the issue in upstream
issue matplotlib#24092

The test case for bad plot (test_tick_values_not_empty) should fail
until we implement our fix.

The test case for good plot (test_tick_values_correct) should already
suceed and continue to succeed after we implement our fix.
@haval0
Copy link
Owner Author

haval0 commented Mar 1, 2023

Currently, the second test case, which represents the "bad plot" case from the upstream issue, mysteriously passes perfectly (even though we have not yet fixed the bug!).

The code in the test is logically exactly the same as this file, which is the code from the upstream issue to reproduce the bug. The linked python file successfully reproduces the bug, and yields an empty tick_values list, but producing the tick_values list in the exact same way in the added test suddenly works perfectly. Hmmm

@haval0
Copy link
Owner Author

haval0 commented Mar 1, 2023

Output showing that the test passes:

% pytest lib/matplotlib/tests/test_ticker.py::TestLogLocator::test_tick_values_not_empty
========================================== test session starts =========================================
[...]

lib/matplotlib/tests/test_ticker.py .                                                             [100%]

======================================== 1 passed in 0.16s =============================================

"classic_mode" had to be disabled for the test to successfully replicate
the normal behavior of matplotlib. Now the test produces an empty list,
which is what should be fixed.
@haval0
Copy link
Owner Author

haval0 commented Mar 1, 2023

Tests now go as expected after setting "classic_mode" to false in the "bad plot" test.

@haval0
Copy link
Owner Author

haval0 commented Mar 2, 2023

With the simple fix of commenting out the if stride == 1 if-clause, this is the test result:

Would need to look into requirements to see if the test is wrong or if the simple fix gives the wrong result

E           AssertionError:
E           Arrays are not equal
E
E           (shapes (21,), (33,) mismatch)
E            x: array([1.e-02, 2.e-02, 5.e-02, 1.e+00, 2.e+00, 5.e+00, 1.e+02, 2.e+02,
E                  5.e+02, 1.e+04, 2.e+04, 5.e+04, 1.e+06, 2.e+06, 5.e+06, 1.e+08,
E                  2.e+08, 5.e+08, 1.e+10, 2.e+10, 5.e+10])
E            y: array([1.e-01, 2.e-01, 5.e-01, 1.e+00, 2.e+00, 5.e+00, 1.e+01, 2.e+01,
E                  5.e+01, 1.e+02, 2.e+02, 5.e+02, 1.e+03, 2.e+03, 5.e+03, 1.e+04,
E                  2.e+04, 5.e+04, 1.e+05, 2.e+05, 5.e+05, 1.e+06, 2.e+06, 5.e+06,...

/usr/lib/python3.10/contextlib.py:79: AssertionError
============================================ short test summary info ============================================
FAILED lib/matplotlib/tests/test_ticker.py::TestLogLocator::test_tick_values_not_empty - AssertionError:
========================================== 1 failed, 5 passed in 1.24s ==========================================

@haval0
Copy link
Owner Author

haval0 commented Mar 7, 2023

Both tests pass after stride calculation behavior revert in #3 . Should prevent any similar regressions in the future.

@haval0 haval0 marked this pull request as ready for review March 7, 2023 10:53
@haval0 haval0 merged commit 01eff90 into issue/24092 Mar 7, 2023
@haval0 haval0 deleted the test-good-bad-plot branch March 7, 2023 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant