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

threads.tmap() test and and fix. Unsurfaced bug #1768 #1965

Merged
merged 4 commits into from Sep 4, 2020

Conversation

cruxicheiros
Copy link
Contributor

Issue: #1768

Background: threads.tmap() was missing unit tests, so I wrote a unit test to cover the situation where the optional parameter stop_on_error is set to False. The purpose of stop_on_error is to determine whether tmap should return a result or throw an error if one or more of the function calls it makes throw exceptions.

Description: As part of my unit test's setup, it calls tmap with the stop_on_error flag set to False. However, an exception is thrown by tmap during this process because the resulting FuncResults don't have the result attribute they are expected to.

Caveats: The actual form of the unit test may need to change depending on how the issue preventing it from running is resolved. It needs to be determined what the actual correct result should be in order to write the unit test.

Finally, this is the stack trace I got before.

Error
Traceback (most recent call last):
  File "/home/clark/miniconda3/envs/pygamenv/lib/python3.8/unittest/case.py", line 60, in testPartExecutor
    yield
  File "/home/clark/miniconda3/envs/pygamenv/lib/python3.8/unittest/case.py", line 676, in run
    self._callTestMethod(testMethod)
  File "/home/clark/miniconda3/envs/pygamenv/lib/python3.8/unittest/case.py", line 633, in _callTestMethod
    method()
  File "/home/clark/Dev/pygame/pygame/test/threads_test.py", line 136, in test_tmap
    tmapped2 = list(tmap(always_excepts, data2, stop_on_error=False))
  File "/home/clark/miniconda3/envs/pygamenv/lib/python3.8/site-packages/pygame/threads/__init__.py", line 308, in <lambda>
    return map(lambda x:x.result, results)
AttributeError: 'FuncResult' object has no attribute 'result'


Assertion failed

Assertion failed


Ran 1 test in 0.036s

FAILED (errors=1)

This is my first PR to this project, so let me know if I have made mistakes in formatting it or if you need more information. Thanks!

@cruxicheiros
Copy link
Contributor Author

Looks like the same thing has happened in the travis build, which at least is good for reproducibility

======================================================================

ERROR: test_tmap (pygame.tests.threads_test.ThreadsModuleTest)

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/opt/python/cp38-cp38/lib/python3.8/site-packages/pygame/tests/threads_test.py", line 136, in test_tmap

    tmapped2 = list(tmap(always_excepts, data2, stop_on_error=False))

  File "/opt/python/cp38-cp38/lib/python3.8/site-packages/pygame/threads/__init__.py", line 308, in <lambda>

    return map(lambda x:x.result, results)

AttributeError: 'FuncResult' object has no attribute 'result'

----------------------------------------------------------------------

Ran 1478 tests in 22.190s

FAILED (errors=1)

Makefile:4: recipe for target

@MyreMylar
Copy link
Contributor

I guess we need to resolve the issue with tmap before anything can be done with this unit test.

@cruxicheiros
Copy link
Contributor Author

Yeah, that's what I think. Should I switch this to a draft and make a new issue?

@MyreMylar
Copy link
Contributor

Yeah, that's what I think. Should I switch this to a draft and make a new issue?

Sounds sensible to me.

@cruxicheiros
Copy link
Contributor Author

@MyreMylar Here: #1985

@cruxicheiros
Copy link
Contributor Author

Modified tmap per issue #1985

@MyreMylar
Copy link
Contributor

Cool.

Are you intending to look at the unfinished unit test that starts with todo in this PR? This one:

pygame/test/threads_test.py

Lines 131 to 143 in 7d3900d

def todo_test_tmap__None_func_and_multiple_sequences(self):
"""Using a None as func and multiple sequences"""
self.fail()
res = tmap(None, [1, 2, 3, 4])
res2 = tmap(None, [1, 2, 3, 4], [22, 33, 44, 55])
res3 = tmap(None, [1, 2, 3, 4], [22, 33, 44, 55, 66])
res4 = tmap(None, [1, 2, 3, 4, 5], [22, 33, 44, 55])
self.assertEqual([1, 2, 3, 4], res)
self.assertEqual([(1, 22), (2, 33), (3, 44), (4, 55)], res2)
self.assertEqual([(1, 22), (2, 33), (3, 44), (4, 55), (None, 66)], res3)
self.assertEqual([(1, 22), (2, 33), (3, 44), (4, 55), (5, None)], res4)

No worries if you are not.

@cruxicheiros
Copy link
Contributor Author

I actually hadn't noticed it and assumed that the main tmap test was the one that was unfinished. I wanted to add further tests to the main tmap test and I can have a look at the existing todo when I do that.

@MightyJosip MightyJosip linked an issue Aug 11, 2020 that may be closed by this pull request
@illume
Copy link
Member

illume commented Aug 30, 2020

I think we can merge this one in now. Further work on tmap tests can happen in other PRs.

@illume illume marked this pull request as ready for review August 30, 2020 10:46
Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :)

@MyreMylar MyreMylar merged commit d770669 into pygame:master Sep 4, 2020
@illume illume changed the title Added unit test for threads.tmap(), unsurfaced bug #1768 threads.tmap() test, unsurfaced bug #1768 Sep 14, 2020
@illume illume added the threads label Sep 14, 2020
@illume illume changed the title threads.tmap() test, unsurfaced bug #1768 threads.tmap() test and and fix. Unsurfaced bug #1768 Sep 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add unit test: threads.tmap()
3 participants