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

[BEAM-1251] Unskip passing Python 3 tests #7446

Merged
merged 2 commits into from
Feb 12, 2019

Conversation

RobbeSneyders
Copy link
Contributor

This is is part of a series of PRs with goal to make Apache Beam PY3 compatible. The proposal with the outlined approach has been documented here: https://s.apache.org/beam-python-3.

This PR removes the skip condition for some tests which were indirectly fixed for Python 3.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status
Build Status
Build Status
Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
Build Status --- --- ---

@RobbeSneyders
Copy link
Contributor Author

Some tests still fail on Jenkins while succeeding locally. I'll look into this.

@RobbeSneyders
Copy link
Contributor Author

@tvalentyn @markflyhigh
I checked this locally, and it seems to fail due to the Python 3 version. The tests fail on Python 3.5.2 and pass on 3.5.6. There were some changes to the typing library between these releases.
Jenkins is currently running Python 3.5.2. Can we upgrade this to 3.5.6?

@tvalentyn
Copy link
Contributor

Asked on https://issues.apache.org/jira/browse/INFRA-17335 to upgrade the interpreter. Sounds like this may be a limitation we may want to call out. Do we understand the nature of the failures on 3.5.2?

@tvalentyn
Copy link
Contributor

retest this please

@tvalentyn
Copy link
Contributor

(just to take another look on the failure we see).

@tvalentyn
Copy link
Contributor

These tests pass on 3.3. I suggest we skip the passing tests only if Python interpreter version is less than 3.5.3, then we will keep the tests for 3.6, and devs running 3.5.3 or higher on their workstations will still exercise the tests.

@RobbeSneyders
Copy link
Contributor Author

Ok, thank you. Sounds good.
Should we also throw a warning when using the failing functionality on versions <3.5.3?

@tvalentyn
Copy link
Contributor

No, instead of warnings I think we will have to set python_requires to >= 3.5.3 for Python 3 versions.

@@ -38,9 +38,12 @@ class _TestClass(object):


class NativeTypeCompatibilityTest(unittest.TestCase):
@unittest.skipIf(sys.version_info[0] == 3 and

@unittest.skipIf(sys.version_info[0] == 3 and sys.version_info[1] < 6 and
Copy link
Contributor

Choose a reason for hiding this comment

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

sys.version_info[1] < 3?

Copy link
Contributor Author

@RobbeSneyders RobbeSneyders Feb 11, 2019

Choose a reason for hiding this comment

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

Changed to sys.hexversion for easy checking.

Copy link
Contributor

@tvalentyn tvalentyn 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!

@tvalentyn
Copy link
Contributor

@aaltay @charlesccychen to help with the merge, thank you!

Copy link
Contributor

@charlesccychen charlesccychen 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 LGTM.

@charlesccychen charlesccychen merged commit 4374b46 into apache:master Feb 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants