Skip to content
This repository was archived by the owner on Sep 17, 2025. It is now read-only.

Fix unit test failures on Windows#361

Merged
reyang merged 1 commit intomasterfrom
fix_test
Nov 30, 2018
Merged

Fix unit test failures on Windows#361
reyang merged 1 commit intomasterfrom
fix_test

Conversation

@reyang
Copy link
Copy Markdown
Contributor

@reyang reyang commented Oct 25, 2018

The current implementation fails the unit test on Windows, due to the backslashes introduced by os.path.join.
The fix is to replace backslash with forward slash before return.

@reyang reyang requested a review from mayurkale22 October 25, 2018 17:25
@reyang reyang changed the title fix unit test failures on Windows Fix unit test failures on Windows Oct 25, 2018
@reyang reyang requested a review from liyanhui1228 October 25, 2018 17:26
@reyang
Copy link
Copy Markdown
Contributor Author

reyang commented Oct 25, 2018

Looks like the CI environment has been updated which caused the following error:

opencensus/stats/exporters/stackdriver_exporter.py:23:42: W606 'async' and 'await' are reserved keywords starting with Python 3.7
opencensus/stats/exporters/stackdriver_exporter.py:112:28: W606 'async' and 'await' are reserved keywords starting with Python 3.7
opencensus/stats/exporters/stackdriver_exporter.py:450:5: F841 local variable 'exception' is assigned to but never used
opencensus/trace/propagation/google_cloud_format.py:22:17: W605 invalid escape sequence '/'
opencensus/trace/propagation/google_cloud_format.py:22:40: W605 invalid escape sequence '\d'

I'll try to fix them.

@reyang
Copy link
Copy Markdown
Contributor Author

reyang commented Oct 25, 2018

I've submitted a separate PR #362 to unblock the build.

return os.path.join("custom.googleapis.com", "opencensus", view_name)
return os.path.join(metric_prefix, view_name)
metric_prefix = metric_prefix or "custom.googleapis.com/opencensus"
return os.path.join(metric_prefix, view_name).replace('\\', '/')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

using os.path.join here may not be ideal. Perhaps use join with the specified separator instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

os.path.join will take care of duplicated/trailing separators, for example, join("a/b/", "c") will yield the same result as join("a/b", "c"). It'll be better to reuse such logic from os.path.join than manually doing path normalization and joining.

@reyang reyang merged commit 0e5f67c into master Nov 30, 2018
@reyang
Copy link
Copy Markdown
Contributor Author

reyang commented Nov 30, 2018

Thanks @bogdandrutu and @SergeyKanzhelev.

@reyang reyang deleted the fix_test branch December 3, 2018 19:04
@reyang reyang mentioned this pull request May 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants