Skip to content

[BEAM-1504] Parameterized tests for standard coders in Python SDK#2028

Closed
vikkyrk wants to merge 4 commits intoapache:masterfrom
vikkyrk:parameterized_tests
Closed

[BEAM-1504] Parameterized tests for standard coders in Python SDK#2028
vikkyrk wants to merge 4 commits intoapache:masterfrom
vikkyrk:parameterized_tests

Conversation

@vikkyrk
Copy link
Contributor

@vikkyrk vikkyrk commented Feb 17, 2017

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

python setup.py test -s apache_beam.coders.standard_coders_test

...

test_standard_coder_0_bytes (apache_beam.coders.standard_coders_test.StandardCodersTest) ... ok test_standard_coder_1_bytes (apache_beam.coders.standard_coders_test.StandardCodersTest) ... ok test_standard_coder_2_varint (apache_beam.coders.standard_coders_test.StandardCodersTest) ... ok test_standard_coder_3_kv (apache_beam.coders.standard_coders_test.StandardCodersTest) ... ok test_standard_coder_4_kv (apache_beam.coders.standard_coders_test.StandardCodersTest) ... ok test_standard_coder_5_kv (apache_beam.coders.standard_coders_test.StandardCodersTest) ... ok test_standard_coder_6_intervalwindow (apache_beam.coders.standard_coders_test.StandardCodersTest) ... ok test_standard_coder_7_stream (apache_beam.coders.standard_coders_test.StandardCodersTest) ... ok test_standard_coder_8_stream (apache_beam.coders.standard_coders_test.StandardCodersTest) ... ok

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify. (Even better, enable
    Travis-CI on your fork and ensure the whole test matrix passes).
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

@vikkyrk
Copy link
Contributor Author

vikkyrk commented Feb 17, 2017

R: @aaltay
CC: @dhalperi @robertwb

@dhalperi
Copy link
Contributor

Nice, thanks.

@asfbot
Copy link

asfbot commented Feb 17, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7510/

Build result: FAILURE

[...truncated 10851 lines...] at hudson.remoting.UserRequest.perform(UserRequest.java:50) at hudson.remoting.Request$2.run(Request.java:332) at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.rat.mp.RatCheckException: Too many files with unapproved license: 8 See RAT report in: /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/sdks/python/target/beam-sdks-python-0.6.0-SNAPSHOT.rat at org.apache.rat.mp.RatCheckMojo.check(RatCheckMojo.java:183) at org.apache.rat.mp.RatCheckMojo.execute(RatCheckMojo.java:156) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 more2017-02-17T01:41:12.407 [ERROR] 2017-02-17T01:41:12.407 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-02-17T01:41:12.407 [ERROR] 2017-02-17T01:41:12.407 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-02-17T01:41:12.407 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException2017-02-17T01:41:12.408 [ERROR] 2017-02-17T01:41:12.408 [ERROR] After correcting the problems, you can resume the build with the command2017-02-17T01:41:12.408 [ERROR] mvn -rf :beam-sdks-pythonchannel stoppedSetting status of bb7e91b to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7510/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

@vikkyrk
Copy link
Contributor Author

vikkyrk commented Feb 17, 2017

R: @jasonkuster for pom changes.

@asfbot
Copy link

asfbot commented Feb 17, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7517/

Build result: FAILURE

[...truncated 10902 lines...] at hudson.remoting.UserRequest.perform(UserRequest.java:50) at hudson.remoting.Request$2.run(Request.java:332) at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.rat.mp.RatCheckException: Too many files with unapproved license: 55 See RAT report in: /home/jenkins/jenkins-slave/workspace/beam_PreCommit_Java_MavenInstall/sdks/python/target/beam-sdks-python-0.6.0-SNAPSHOT.rat at org.apache.rat.mp.RatCheckMojo.check(RatCheckMojo.java:183) at org.apache.rat.mp.RatCheckMojo.execute(RatCheckMojo.java:156) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 more2017-02-17T03:06:52.912 [ERROR] 2017-02-17T03:06:52.912 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-02-17T03:06:52.912 [ERROR] 2017-02-17T03:06:52.912 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-02-17T03:06:52.912 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException2017-02-17T03:06:52.913 [ERROR] 2017-02-17T03:06:52.913 [ERROR] After correcting the problems, you can resume the build with the command2017-02-17T03:06:52.913 [ERROR] mvn -rf :beam-sdks-pythonchannel stoppedSetting status of 1e58610 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7517/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.003%) to 69.22% when pulling b4f4751 on vikkyrk:parameterized_tests into 2ca3bf6 on apache:master.

@asfbot
Copy link

asfbot commented Feb 17, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7520/
--none--

Copy link
Member

@aaltay aaltay left a comment

Choose a reason for hiding this comment

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

I think this is great.

Have you verified general test scenarios likes tox, setup.py test, nosetest single test etc. and see that these tests are included.



REQUIRED_TEST_PACKAGES = [
'nose-parameterized>=0.5.0',
Copy link
Member

Choose a reason for hiding this comment

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

Could you upper bound this with <0.6.0

os.path.dirname(__file__), '..', 'tests', 'data', 'standard_coders.yaml')


def data(test_yaml):
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment about what this is doing and use a less generic name for the function.

@asfbot
Copy link

asfbot commented Feb 21, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7624/

Build result: FAILURE

[...truncated 1.22 MB...] at java.lang.Thread.run(Thread.java:745)Caused by: org.apache.maven.plugin.MojoExecutionException: Command execution failed. at org.codehaus.mojo.exec.ExecMojo.execute(ExecMojo.java:302) at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:134) at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208) ... 31 moreCaused by: org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) at org.apache.commons.exec.DefaultExecutor.executeInternal(DefaultExecutor.java:404) at org.apache.commons.exec.DefaultExecutor.execute(DefaultExecutor.java:166) at org.codehaus.mojo.exec.ExecMojo.executeCommandLine(ExecMojo.java:764) at org.codehaus.mojo.exec.ExecMojo.executeCommandLine(ExecMojo.java:711) at org.codehaus.mojo.exec.ExecMojo.execute(ExecMojo.java:289) ... 33 more2017-02-21T06:37:57.027 [ERROR] 2017-02-21T06:37:57.027 [ERROR] Re-run Maven using the -X switch to enable full debug logging.2017-02-21T06:37:57.027 [ERROR] 2017-02-21T06:37:57.027 [ERROR] For more information about the errors and possible solutions, please read the following articles:2017-02-21T06:37:57.027 [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException2017-02-21T06:37:57.027 [ERROR] 2017-02-21T06:37:57.027 [ERROR] After correcting the problems, you can resume the build with the command2017-02-21T06:37:57.027 [ERROR] mvn -rf :beam-sdks-pythonchannel stoppedSetting status of fcd8b537b6bfff034205f16d423badb720888629 to FAILURE with url https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7624/ and message: 'Build finished. 'Using context: Jenkins: Maven clean install
--none--

@vikkyrk
Copy link
Contributor Author

vikkyrk commented Feb 21, 2017

Addressed your comments, and yes it works with python setup.py test, tox etc.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 69.18% when pulling 6457dbe on vikkyrk:parameterized_tests into 8cfb3d1 on apache:master.

@asfbot
Copy link

asfbot commented Feb 21, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7626/
--none--

Copy link
Member

@aaltay aaltay left a comment

Choose a reason for hiding this comment

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

LGTM. I added one more comment, I can merge it after that.



REQUIRED_TEST_PACKAGES = [
'nose-parameterized>=0.5.0, <0.6.0',
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove the space between , and <?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 69.162% when pulling 2b70f9d on vikkyrk:parameterized_tests into 8cfb3d1 on apache:master.

@asfbot
Copy link

asfbot commented Feb 21, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7638/
--none--

@asfgit asfgit closed this in eb59a1d Feb 21, 2017
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.

5 participants