Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use set for deck_names container to prevent the test from being nonde… #21

Closed
wants to merge 1 commit into from

Conversation

ckp95
Copy link
Contributor

@ckp95 ckp95 commented Jun 7, 2020

When I ran pytest I noticed that the tests would pass sometimes and fail others. This turned out to be because the order of a.deck_names would change from one run to the next:


[ckp95@ckp95-desktop apy]$ pytest
================================== test session starts ==================================
platform linux -- Python 3.8.3, pytest-5.4.3, py-1.8.1, pluggy-0.13.1
rootdir: /home/ckp95/Documents/apy, inifile: pytest.ini
collected 7 items                                                                       

tests/test_basics.py ...                                                          [ 42%]
tests/test_batch_edit.py .                                                        [ 57%]
tests/test_decks.py .                                                             [ 71%]
tests/test_errors.py .                                                            [ 85%]
tests/test_models.py .                                                            [100%]

=================================== 7 passed in 0.76s ===================================
[ckp95@ckp95-desktop apy]$ pytest
================================== test session starts ==================================
platform linux -- Python 3.8.3, pytest-5.4.3, py-1.8.1, pluggy-0.13.1
rootdir: /home/ckp95/Documents/apy, inifile: pytest.ini
collected 7 items                                                                       

tests/test_basics.py ...                                                          [ 42%]
tests/test_batch_edit.py .                                                        [ 57%]
tests/test_decks.py F                                                             [ 71%]
tests/test_errors.py .                                                            [ 85%]
tests/test_models.py .                                                            [100%]

======================================= FAILURES ========================================
______________________________________ test_decks _______________________________________

    def test_decks():
        """Test empty collection"""
        with AnkiSimple() as a:
            assert a.col.decks.count() == 2
            assert a.col.decks.current()['name'] == 'NewDeck'
>           assert list(a.deck_names) == ['Default', 'NewDeck']
E           AssertionError: assert ['NewDeck', 'Default'] == ['Default', 'NewDeck']
E             At index 0 diff: 'NewDeck' != 'Default'
E             Use -v to get the full diff

tests/test_decks.py:14: AssertionError
================================ short test summary info ================================
FAILED tests/test_decks.py::test_decks - AssertionError: assert ['NewDeck', 'Default']...
============================== 1 failed, 6 passed in 0.88s ==============================

I couldn't figure out why this would be -- I looked around in the anki source (decks.py) and it seems to be stored as a dict, not a set, and dicts have been guaranteed to preserve order since Python 3.7.

Regardless, I changed list to set so that the order doesn't matter for the test, now it passes all the time.

@lervag
Copy link
Owner

lervag commented Jun 7, 2020

Thanks, this is useful and clearly solves an issue with the tests. Much appreciated!

lervag added a commit that referenced this pull request Jun 7, 2020
Use `set` for deck_names container to prevent nondeterministic test
error.

refer: #21
@lervag lervag closed this Jun 7, 2020
@ckp95 ckp95 deleted the nondeterministic-test-fix branch June 8, 2020 11:42
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.

2 participants