Skip to content

AVRO-2652: Use Tox to Enable Multi Python Testing#742

Merged
kojiromike merged 6 commits intoapache:masterfrom
kojiromike:AVRO-2652/tox
Dec 19, 2019
Merged

AVRO-2652: Use Tox to Enable Multi Python Testing#742
kojiromike merged 6 commits intoapache:masterfrom
kojiromike:AVRO-2652/tox

Conversation

@kojiromike
Copy link
Copy Markdown
Contributor

@kojiromike kojiromike commented Dec 11, 2019 •
edited
Loading

Jira

Tests

  • My PR changes how we run tests, but doesn't really change the tests.

Commits

Documentation

  • My PR contains no documentation changes because it does not change how the avro implementation functions.

@kojiromike kojiromike self-assigned this Dec 11, 2019
@Fokko
Copy link
Copy Markdown
Contributor

Fokko commented Dec 11, 2019

I never understood why we need tox when we have Docker. I understand that we now have one single environment for everything which isn't ideal.

@kojiromike
Copy link
Copy Markdown
Contributor Author

kojiromike commented Dec 12, 2019 •
edited
Loading

By itself tox doesn't do much with only a single python version, and if we had a bunch of different Dockerfiles it might be a different story, but in this case, with python 2.7 and 3.5 installed in the same image, tox will be pretty useful. I am laying a groundwork here to be able to run multiple python versions against the same lang/py tests to show that we have effective python 3.5 support and can deprecate lang/py3.

@kojiromike
Copy link
Copy Markdown
Contributor Author

kojiromike commented Dec 12, 2019

Another useful thing tox does is actually run the tests against the installable package, instead of testing against just a bunch of files. I found several bugs in our packaging setup just by trying to enable tox.

@kojiromike kojiromike changed the title Avro 2652: Use Tox to Enable Multi Python Testing AVRO-2652: Use Tox to Enable Multi Python Testing Dec 16, 2019
@kojiromike kojiromike mentioned this pull request Dec 16, 2019
Merged
7 tasks
RyanSkraba
RyanSkraba approved these changes Dec 17, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@RyanSkraba RyanSkraba left a comment

Choose a reason for hiding this comment

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

Hello! This LGTM, but was mostly a tox learning experience for me (although thankfully the docs are a bit lighter after going through setuptools!

This builds and tests correctly (except for the problem already noted in #746).

@kojiromike
AVRO-2652: Fix Inconsistent Pretty Output Order
3e67b24 
Using json.dumps on a dict gives the simplest possible way to
pretty-print a schema, but it doesn't give any control over the output
order. But the test kit assumes the output is in a specific order, which
is not guaranteed by older Python implementations. This change sorts the
keys for the simplest possible stable output order.
@kojiromike
AVRO-2652: Run Tests with Tox
51da9f5 
Tox enables testing with multiple versions of python in the same place.
@kojiromike
AVRO-2652: Tox in Docker
6c9c16f
@kojiromike
AVRO-2657: Fix Python Interop Data Generator
6349592
@kojiromike kojiromike mentioned this pull request Dec 18, 2019
Closed
7 tasks
@RyanSkraba
Copy link
Copy Markdown
Contributor

RyanSkraba commented Dec 18, 2019

The additional fix for AVRO-2657 works for me (in the build logs:)

+ cd lang/py
+ ./build.sh interop-data-test
TEST INTEROP
============
READING perl_deflate.avro
READING perl_zstandard.avro
READING perl.avro
READING csharp_deflate.avro
<snip>
READING py_zstandard.avro
READING py_snappy.avro
READING py_deflate.avro
...

kojiromike
kojiromike commented Dec 18, 2019
View reviewed changes
lang/py/tox.ini
mkdir -p avro/test/interop {toxinidir}/../../build/interop/data
cp -r {toxinidir}/../../build/interop/data avro/test/interop
python -m avro.test.gen_interop_data avro/interop.avsc avro/test/interop/data/py.avro
cp -r avro/test/interop/data {toxinidir}/../../build/interop
Copy link
Copy Markdown
Contributor Author

@kojiromike kojiromike Dec 18, 2019

Choose a reason for hiding this comment

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

@sekikn @RyanSkraba I've rolled the AVRO-2657 fix into this PR, using slightly different implementations in build.sh and tox.ini. You can see here the precommand always does a best-effort to get any avro generated by other languages' interop generators into the python data path. Unlike ./build.sh interop-data-test, this approach will test with any environment that we can describe to tox (see #744). Here, the avro files don't get copied into the working copy, but only into the tox virtualenv.

Do you have any feedback on this approach?

@kojiromike kojiromike merged commit bd9e6c4 into apache:master Dec 19, 2019
@kojiromike kojiromike deleted the AVRO-2652/tox branch December 19, 2019 02:30
@sekikn
Copy link
Copy Markdown
Contributor

sekikn commented Dec 19, 2019

@kojiromike Sorry for the late reply, and failing CI after merging this PR is my bad. Merging #705 is supposed to resolve it.

@kojiromike
Copy link
Copy Markdown
Contributor Author

kojiromike commented Dec 19, 2019

That's ok. I merged the other pr. Looking forward to being green again!

@sekikn
Copy link
Copy Markdown
Contributor

sekikn commented Dec 19, 2019

I saw the CI went back to green. Thanks @kojiromike!

@Fokko
Copy link
Copy Markdown
Contributor

Fokko commented Dec 21, 2019

Sorry for the late reply. On the second thought, I think having tox in this case makes sense. I don't see any other pragmatic options.

ecopoesis pushed a commit to ecopoesis/avro that referenced this pull request Jan 8, 2020
@kojiromike @ecopoesis
AVRO-2652: Use Tox to Enable Multi Python Testing (apache#742)
b08de83 
* AVRO-2652: Fix Inconsistent Pretty Output Order

Using json.dumps on a dict gives the simplest possible way to
pretty-print a schema, but it doesn't give any control over the output
order. But the test kit assumes the output is in a specific order, which
is not guaranteed by older Python implementations. This change sorts the
keys for the simplest possible stable output order.

* AVRO-2652: Run Tests with Tox

Tox enables testing with multiple versions of python in the same place.

* AVRO-2652: Tox in Docker

* AVRO-2657: Fix Python Interop Data Generator

* AVRO-2657: Test with Interop Data

* AVRO-2657: Make build.sh do interop tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RyanSkraba RyanSkraba RyanSkraba approved these changes

Assignees

@kojiromike kojiromike

Labels

build Python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kojiromike @Fokko @RyanSkraba @sekikn