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

bpo-39299: Add more tests for mimetypes and its cli. #17949

Open
wants to merge 8 commits into
base: master
from

Conversation

@tirkarthi
Copy link
Contributor

tirkarthi commented Jan 11, 2020

  • Add test for case insensitive check of types and extensions.
  • Add test for data url with no comma.
  • Add test for read_mime_types.
  • Add tests for the mimetypes cli.

https://bugs.python.org/issue39299

@tirkarthi tirkarthi requested a review from python/email-team as a code owner Jan 11, 2020
@tirkarthi

This comment has been minimized.

Copy link
Contributor Author

tirkarthi commented Jan 11, 2020

Ah, the logic to load the initial database is with known types, common types and the list of files from mimetypes.knownfiles subsequently overriding each other. At the start of the test mimetypes.knownfiles is set as empty list but since I am using script_helper.assert_python_ok setting the list of knownfiles to empty list doesn't work and mac seems to load a mimetype for .pic that is different from common types and overrides it.

I guess my tests can be rewritten to make sure the tests are not machine dependent based on the files.

* Set knownfiles for mimetypes to be empty list only during setup so that it doesn't leak into other tests.
* Move the __main__ code to separate function for better testing.
@tirkarthi

This comment has been minimized.

Copy link
Contributor Author

tirkarthi commented Jan 11, 2020

I have moved the code for __main__ to be a private function to help testing by directly calling it instead of using subprocess to invoke python -m mimetypes. I have also found that mimetypes.knownfiles was set as empty at the top of the module. It has a potential side effect that all other tests that follow after test_mimetypes will also have empty list of files when accessing mimetypes.knownfiles. I have moved it to setup call so that this change doesn't affect other tests.

@tirkarthi

This comment has been minimized.

Copy link
Contributor Author

tirkarthi commented Jan 11, 2020

I am slightly confused since importing mimetypes itself sets inited as False and also calls _default_mime_types to set global values so I removed them from the test since it's already done. In the test when I call the _default_mime_types which sets the global variables the value for suffix, encoding and types maps it poses a strange failure for windows test which copies the types_map and clears it to reinitialize it again where _types_map[True] is empty. I need to figure out yet over why the call has a side effect.

@tirkarthi

This comment has been minimized.

Copy link
Contributor Author

tirkarthi commented Jan 11, 2020

I resorted to using setUpModule and tearDownModule to have the original test code and not to use mixins for the setup. This will restore knownfiles value in the end. The test coverage with this PR stands at 80% with the code for windows not as part of the coverage report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.