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

Fix multiprocessing tests on macOS. #116

Merged
merged 1 commit into from Jul 18, 2023
Merged

Fix multiprocessing tests on macOS. #116

merged 1 commit into from Jul 18, 2023

Conversation

tjni
Copy link
Contributor

@tjni tjni commented Jul 17, 2023

On macOS, the default method for creating a new process is to use spawn instead of fork. When this occurs, arguments to the process target need to be pickled, but the test includes several unpicklable objects, such as local functions and mmaps.

We can fix the test's current assumptions by forcing "fork" instead of "spawn". I'm also open to fixing the test in other ways if we want to test that spawn works as well.

For reference, the original error signature:

obj = <Process name='Process-129' parent=38670 initial>, file = <_io.BytesIO object at 0x102294450>, protocol = None

    def dump(obj, file, protocol=None):
        '''Replacement for pickle.dump() using ForkingPickler.'''
>       ForkingPickler(file, protocol).dump(obj)
E       AttributeError: Can't pickle local object 'BaseTestReader._check_concurrency.<locals>.lookup'

On macOS, the default method for creating a new process is to use spawn
instead of fork. When this occurs, arguments to the process target need
to be pickled, but the test includes several unpicklable objects, such
as local functions and mmaps.

We can fix the test's current assumptions by forcing "fork" instead of
"spawn".
Copy link
Member

@oschwald oschwald left a comment

Choose a reason for hiding this comment

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

Thanks! This seems like a good immediate fix and seems to match the original intent of the test.

@oschwald oschwald merged commit b511290 into maxmind:main Jul 18, 2023
9 checks passed
@tjni tjni deleted the fix-multiprocessing-test-mac branch July 18, 2023 16:35
oschwald added a commit that referenced this pull request Jul 18, 2023
@oschwald oschwald mentioned this pull request Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants