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

Don't mock some os functions on windows #1912

Merged
merged 1 commit into from Apr 30, 2020

Conversation

vijayphoenix
Copy link
Contributor

@vijayphoenix vijayphoenix commented Apr 19, 2020

Fix #817

Related #1911 (comment)

@googlebot googlebot added the cla: yes Author has signed CLA label Apr 19, 2020
@Eshan-Agarwal
Copy link
Contributor

@vijayphoenix Can you check if it works properly now ?. It's still shows me same error after do modification as in this PR

Traceback (most recent call last):
File "c:\users\eshan\desktop\tf_tfds\datasets\tensorflow_datasets\testing\dataset_builder_testing.py", line 166, in setUp
self._mock_out_forbidden_os_functions()
File "c:\users\eshan\desktop\tf_tfds\datasets\tensorflow_datasets\testing\dataset_builder_testing.py", line 185, in _mock_out_forbidden_os_functions
getattr(mock_os, fop).side_effect = err
File "C:\Users\eshan\Anaconda3\envs\keras-gpu\lib\unittest\mock.py", line 585, in getattr
raise AttributeError("Mock object has no attribute %r" % name)
AttributeError: Mock object has no attribute 'chown'

@tfds-bot tfds-bot added the community:please_review Community - We need your help to review this PR. label Apr 19, 2020
@vijayphoenix
Copy link
Contributor Author

I think you have not included the changes in the proper location file as your stack trace corresponds to the old file.

@Eshan-Agarwal
Copy link
Contributor

@vijayphoenix earlier was my mistake I change in wrong files but still got same error after do proper changes

Traceback (most recent call last):
File "c:\users\eshan\desktop\tf_tfds\datasets\tensorflow_datasets\testing\dataset_builder_testing.py", line 166, in setUp
self._mock_out_forbidden_os_functions()
File "c:\users\eshan\desktop\tf_tfds\datasets\tensorflow_datasets\testing\dataset_builder_testing.py", line 185, in _mock_out_forbidden_os_functions
getattr(mock_os, fop).side_effect = err
File "C:\Users\eshan\Anaconda3\envs\keras-gpu\lib\unittest\mock.py", line 585, in getattr
raise AttributeError("Mock object has no attribute %r" % name)
AttributeError: Mock object has no attribute 'chown'

@vijayphoenix
Copy link
Contributor Author

The stack trace is still the same (the line no. should change as the file was modified).

To avoid these types of problems, I suggest you make a new env.

@Eshan-Agarwal
Copy link
Contributor

@vijayphoenix Yeah right actually I messed with some files its working great :-)

Copy link
Member

@Conchylicultor Conchylicultor left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this.

Note internally, I've replaced this code by:

    for fop in FORBIDDEN_OS_FUNCTIONS:
      if os.name == "nt" and not hasattr(os, fop):
        continue  # Not all os functions are available on Windows.
      ...

It should be more generic, rather than hardcoding specific values. However I don't have access to windows machine so I can't really test this.

@tfds-bot tfds-bot added kokoro:run Run Kokoro tests and removed community:please_review Community - We need your help to review this PR. labels Apr 30, 2020
@kokoro-team kokoro-team removed the kokoro:run Run Kokoro tests label Apr 30, 2020
@vijayphoenix
Copy link
Contributor Author

Tested; works on my machine.

@tfds-bot tfds-bot added the tfds:ready_to_merge This PR is ready to be merged. label Apr 30, 2020
@tfds-copybara tfds-copybara merged commit cfd58c4 into tensorflow:master Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Author has signed CLA tfds:ready_to_merge This PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unit testing does not work on Windows
7 participants