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-3713] Move PerformanceTest and CrossLanguageValidateRunner from nosetest to pytest #14795

Merged
merged 12 commits into from Jun 17, 2021

Conversation

benWize
Copy link
Contributor

@benWize benWize commented May 12, 2021

Move integration tests from nose to pytest on the following jobs:

  • job_PerformanceTests_Python
  • job_PostCommit_CrossLanguageValidatesRunner_Dataflow

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

ValidatesRunner compliance status (on master branch)

Lang ULR Dataflow Flink Samza Spark Twister2
Go --- Build Status Build Status --- Build Status ---
Java Build Status Build Status
Build Status
Build Status
Build Status
Build Status
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
Build Status
--- Build Status ---
XLang Build Status Build Status Build Status --- Build Status ---

Examples testing status on various runners

Lang ULR Dataflow Flink Samza Spark Twister2
Go --- --- --- --- --- --- ---
Java --- Build Status
Build Status
Build Status
--- --- --- --- ---
Python --- --- --- --- --- --- ---
XLang --- --- --- --- --- --- ---

Post-Commit SDK/Transform Integration Tests Status (on master branch)

Go Java Python
Build Status Build Status Build Status
Build Status
Build Status

Pre-Commit Tests Status (on master branch)

--- Java Python Go Website Whitespace Typescript
Non-portable Build Status
Build Status
Build Status
Build Status
Build Status
Build Status Build Status Build Status Build Status
Portable --- Build Status Build Status --- --- ---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests

See CI.md for more information about GitHub Actions CI.

@benWize
Copy link
Contributor Author

benWize commented May 12, 2021

Run XVR_Dataflow PostCommit

@codecov
Copy link

codecov bot commented May 12, 2021

Codecov Report

Merging #14795 (3fe2dca) into master (00eb420) will increase coverage by 0.21%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14795      +/-   ##
==========================================
+ Coverage   83.78%   83.99%   +0.21%     
==========================================
  Files         435      439       +4     
  Lines       58423    60014    +1591     
==========================================
+ Hits        48948    50408    +1460     
- Misses       9475     9606     +131     
Impacted Files Coverage Δ
.../examples/snippets/transforms/elementwise/pardo.py
...he_beam/io/flink/flink_streaming_impulse_source.py
...hon/apache_beam/runners/direct/direct_userstate.py
..._beam/io/gcp/internal/clients/bigquery/__init__.py
.../python/apache_beam/runners/worker/statesampler.py
...n/apache_beam/runners/interactive/cache_manager.py
...on/apache_beam/runners/direct/helper_transforms.py
...n/apache_beam/examples/complete/game/user_score.py
...38/build/srcs/sdks/python/apache_beam/io/fileio.py
...thon/apache_beam/io/azure/blobstoragefilesystem.py
... and 864 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00eb420...3fe2dca. Read the comment docs.

@benWize
Copy link
Contributor Author

benWize commented May 12, 2021

Run XVR_Dataflow PostCommit

@aaltay
Copy link
Member

aaltay commented May 27, 2021

What is the next step on this PR?

@benWize
Copy link
Contributor Author

benWize commented May 28, 2021

Hi @aaltay!
Some of these tests failed on pytest, I need to investigate further why it is happening. I'll retake this.

@benWize
Copy link
Contributor Author

benWize commented May 29, 2021

Run XVR_Dataflow PostCommit

1 similar comment
@benWize
Copy link
Contributor Author

benWize commented May 29, 2021

Run XVR_Dataflow PostCommit

@benWize benWize changed the title [BEAM-3713][WIP] Move PerformanceTest and CrossLanguageValidateRunner from nosetest to pytest [BEAM-3713] Move PerformanceTest and CrossLanguageValidateRunner from nosetest to pytest May 31, 2021
@benWize benWize marked this pull request as ready for review May 31, 2021 16:49
@benWize
Copy link
Contributor Author

benWize commented May 31, 2021

R: @udim, Hi Udi, would you help me to review this?
I left the nosetests prefix to be able to see the JUnit file before merging.
There is a successful run of PostCommit_CrossLanguageValidatesRunner_Dataflow
I think in order to test PerformanceTests_Python it'll be necessary to run a seed job because the test reference is in the Jenkins file

test : "apache_beam/examples/wordcount_it_test.py::WordCountIT::test_wordcount_it",

"test_opts": config.pytestOptions,
"suite": "xlangValidateRunner",
"pytest": true, // TODO(BEAM-3713): Remove this once nose is removed.
// TODO(BEAM-10507): Remove no_xlang_transforms after fixing
Copy link
Member

Choose a reason for hiding this comment

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

I think you can remove this TODO. We will always want to exclude tests marked with no_xlang_transforms.

@@ -36,7 +36,7 @@
PARQUET_WRITE_URN = "beam:transforms:xlang:test:parquet_write"


@attr('UsesCrossLanguageTransforms')
@pytest.mark.xlang_transforms
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, you could remove the no_xlang_transforms marker and just comment out this line with the same TODO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this, I removed the no_xlang_transforms marker and commented out those tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A successful run with changes in marker no_xlang_transforms https://ci-beam.apache.org/job/beam_PostCommit_XVR_Dataflow_PR/23/testReport/

@udim
Copy link
Member

udim commented Jun 2, 2021

Run BigQueryIO Read Performance Test Python

@udim
Copy link
Member

udim commented Jun 2, 2021

The run of PostCommit_CrossLanguageValidatesRunner_Dataflow looks okay. I ran the seed job on my pr and started a random performance test: https://ci-beam.apache.org/job/beam_PerformanceTests_BiqQueryIO_Read_Python_PR/3/

@udim
Copy link
Member

udim commented Jun 2, 2021

Run Python37 WordCountIT Performance Test

@udim
Copy link
Member

udim commented Jun 2, 2021

https://ci-beam.apache.org/job/beam_PerformanceTests_WordCountIT_Py37/2959/console failed. I think the pytest timeout of 600s might be the issue

@benWize
Copy link
Contributor Author

benWize commented Jun 2, 2021

https://ci-beam.apache.org/job/beam_PerformanceTests_WordCountIT_Py37/2959/console failed. I think the pytest timeout of 600s might be the issue

I think the same, I increased the timeout

@benWize benWize requested a review from udim June 2, 2021 18:12
@benWize
Copy link
Contributor Author

benWize commented Jun 9, 2021

Run XVR_Dataflow PostCommit

@benWize
Copy link
Contributor Author

benWize commented Jun 9, 2021

Hi @udim, would you help me to retest the wordcountIT performance test?

@tysonjh
Copy link
Contributor

tysonjh commented Jun 11, 2021

Run Python37 WordCountIT Performance Test

1 similar comment
@udim
Copy link
Member

udim commented Jun 15, 2021

Run Python37 WordCountIT Performance Test

@udim
Copy link
Member

udim commented Jun 15, 2021

Copy link
Member

@udim udim left a comment

Choose a reason for hiding this comment

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

Looks good, just needs to resolve the temporary TODO(BEAM-3713)s

@benWize
Copy link
Contributor Author

benWize commented Jun 15, 2021

Thanks, Udi, I removed the TODOs.

@udim
Copy link
Member

udim commented Jun 16, 2021

Run Java PreCommit

@udim
Copy link
Member

udim commented Jun 16, 2021

run python precommit

@udim
Copy link
Member

udim commented Jun 17, 2021

Ignoring the precommit failures that seem unrelated

@udim udim merged commit 9418994 into apache:master Jun 17, 2021
@chamikaramj
Copy link
Contributor

Seems like this broke portable xvr tests.

For example,
https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/
https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/1361/console

ModuleNotFoundError: No module named 'pytest'

(Flink and Spark test suites are broken as well)

@benWize
Copy link
Contributor Author

benWize commented Jun 22, 2021

Seems like this broke portable xvr tests.

For example,
https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/
https://ci-beam.apache.org/job/beam_PostCommit_XVR_Direct/1361/console

ModuleNotFoundError: No module named 'pytest'

(Flink and Spark test suites are broken as well)

Fix in #15058

@tvalentyn
Copy link
Contributor

This change might have started to cause test flakiness: https://issues.apache.org/jira/browse/BEAM-12515

@udim
Copy link
Member

udim commented Jun 29, 2021

I don't see how this PR would affect how PipelineOptionsTest.test_display_data runs. It was running with pytest before this PR.

@benWize
Copy link
Contributor Author

benWize commented Jun 30, 2021

I don't think this PR is related to that flakiness, I was able to reproduce it locally in code before this PR was merge using this plugin https://github.com/dropbox/pytest-flakefinder as:
pytest -o junit_suite_name=py38-cloud --junitxml=pytest_py38-cloud.xml --log-cli-level=DEBUG "apache_beam/options/pipeline_options_test.py::PipelineOptionsTest" --flake-finder --flake-runs=10 -n 9.
The problem could be related to xdist, maybe adding the marker @pytest.mark.no_xdist to test_display_data should fix the problem.

@benWize
Copy link
Contributor Author

benWize commented Jun 30, 2021

Possible fix for test_display_data flakiness in #15104

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

6 participants