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.

Sign up for GitHub

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

Jump to bottom

Add sdk test coverage. #1858

Closed
wants to merge 25 commits into from
Closed

Add sdk test coverage. #1858

wants to merge 25 commits into from

Conversation

numerology
Copy link

@numerology numerology commented Aug 15, 2019 •
edited
Loading

Add sdk test coverage collection and report to coveralls.io.

Patched #898

This change is Reviewable

numerology and others added 21 commits July 23, 2019 15:13
@numerology
Remove redundant import.
e0c7132
Simplify sample_test.yaml by using withItem syntax.
70972db
Merge branch 'master' of https://github.com/kubeflow/pipelines
deb6cc0
Simplify sample_test.yaml by using withItem syntax.
da9a42a
Merge branch 'master' of https://github.com/numerology/pipelines
a203039
Change dict to str in withItems.
81ccbf4
Merge branch 'master' of https://github.com/kubeflow/pipelines
d94a945
Merge branch 'master' of https://github.com/kubeflow/pipelines
ee38eb1
Merge branch 'master' of https://github.com/kubeflow/pipelines
9b3ad8e
Merge branch 'master' of https://github.com/kubeflow/pipelines
4ba8f85
Merge branch 'master' of https://github.com/kubeflow/pipelines
2b39278
Merge branch 'master' of https://github.com/kubeflow/pipelines
3061791
Merge remote-tracking branch 'origin/master'
1cc64ec
Merge branch 'master' of https://github.com/kubeflow/pipelines
a42ac40
Merge remote-tracking branch 'origin/master'
2398017
Merge branch 'master' of https://github.com/kubeflow/pipelines
a0bd790
Merge branch 'master' of https://github.com/kubeflow/pipelines
433180c
Merge branch 'master' of https://github.com/kubeflow/pipelines
fcabe77
Patched Ark-kun's PR.
246a178
Revert "Patched Ark-kun's PR."
d043d38 
This reverts commit 246a178
Patched Ark-kun's PR
6ac0ff8
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign paveldournov
You can assign the PR to them by writing /assign @paveldournov in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@numerology numerology mentioned this pull request Aug 16, 2019
Closed
@numerology numerology requested a review from Ark-kun August 16, 2019 04:10
@numerology
Copy link
Author

/retest

@numerology
Copy link
Author

/test kubeflow-pipeline-e2e-test

1 similar comment
@numerology
Copy link
Author

/test kubeflow-pipeline-e2e-test

@numerology numerology changed the title [WIP] Add sdk test coverage. Add sdk test coverage. Aug 16, 2019
@numerology
Copy link
Author

/test all

1 similar comment
@numerology
Copy link
Author

/test all

@Ark-kun
Copy link
Contributor

Ark-kun commented Aug 16, 2019

Can you provide more information as for why --omit */site-packages/* is the correct way to handle the paths?
I'm concerned that this just excludes certain traces from consideration.
I'd like to understand why different traces produce different paths.

P.S. It would be great to make source viewing work. Right now when I click on https://coveralls.io/builds/25178194/source?filename=kfp/components/_components.py I get

SOURCE NOT AVAILABLE
The file "kfp/components/_components.py" isn't available on github. Either it's been removed, or the repo root directory needs to be updated.```

@numerology
Copy link
Author

Can you provide more information as for why --omit */site-packages/* is the correct way to handle the paths?
I'm concerned that this just excludes certain traces from consideration.
I'd like to understand why different traces produce different paths.

P.S. It would be great to make source viewing work. Right now when I click on https://coveralls.io/builds/25178194/source?filename=kfp/components/_components.py I get

SOURCE NOT AVAILABLE
The file "kfp/components/_components.py" isn't available on github. Either it's been removed, or the repo root directory needs to be updated.```

Yes I got it too. Do you think it's okay to solve the problem in a following PR? I suspect that it's because we didn't set 'src' for coveralls correctly.

@numerology
Copy link
Author

/test kubeflow-pipeline-e2e-test

Squashed commit of the following:
d5fd29c 
commit 0ed5819
Author: Yuan (Bob) Gong <gongyuan94@gmail.com>
Date:   Tue Aug 20 08:13:31 2019 +0800

    Refactor presubmit-tests-with-pipeline-deployment.sh to run in other projects (kubeflow#1732)

    * Refactor presubmit-tests-with-pipeline-deployment.sh so that it can be run from a different project

    * Simplify getting service account from cluster.

    * Copy image builder image instead of granting permission

    * Add missed yes command

    * fix stuff

    * Let other usages of image-builder image become configurable

    * let test workflow use image builder image

commit 0369a4c
Author: Ajay Gopinathan <ajaygopinathan@google.com>
Date:   Fri Aug 16 21:02:07 2019 -0700

    Update manifests to point to 0.26 release. (kubeflow#1870)

commit 2b246bc
Author: Alexey Volkov <avolkov@google.com>
Date:   Fri Aug 16 19:54:07 2019 -0700

    SDK - Tests - Improved the "ContainerOp.set_retry" test (kubeflow#1843)

    Properly testing the feature isntead of just comparing with golden data.

commit d66508d
Author: Ajay Gopinathan <ajaygopinathan@google.com>
Date:   Fri Aug 16 18:42:48 2019 -0700

    Update changelog for 0.1.26 (kubeflow#1872)

    * Update changelog for 0.1.26

    * Update changelog.

commit 1d99704
Author: Alexey Volkov <avolkov@google.com>
Date:   Fri Aug 16 18:24:09 2019 -0700

    SDK - Fixed string comparisons (kubeflow#1756)

commit 6d5ffa2
Author: Ajay Gopinathan <ajaygopinathan@google.com>
Date:   Fri Aug 16 16:57:33 2019 -0700

    Remove copying of tfx data for cloudbuild release steps. (kubeflow#1871)

commit de538aa
Author: Ning <ngao@google.com>
Date:   Fri Aug 16 15:53:07 2019 -0700

    add compile step in the samples to generate zip files (kubeflow#1866)

    * add compile step in the samples to generate zip files

commit 5bcc665
Author: Ajay Gopinathan <ajaygopinathan@google.com>
Date:   Fri Aug 16 13:58:23 2019 -0700

    Update Python SDK versions for release. (kubeflow#1860)

commit 0d898cb
Author: Ajay Gopinathan <ajaygopinathan@google.com>
Date:   Fri Aug 16 13:16:11 2019 -0700

    Release 0517114 (kubeflow#1859)

    * Updated component images to version 0517114

    * Updated components to version 48dd338

commit 7dbca1a
Author: Ning <ngao@google.com>
Date:   Fri Aug 16 12:06:37 2019 -0700

    update gcloud ml-engine to ai-platform (kubeflow#1863)

commit e849f22
Author: Ryan Dawson <ryandawson@cantab.net>
Date:   Fri Aug 16 19:21:23 2019 +0100

    Seldon examples (kubeflow#1405)

commit a6f3f40
Author: sina chavoshi <sina.chavoshi@gmail.com>
Date:   Fri Aug 16 09:56:09 2019 -0700

    Adding a sample for serving component (kubeflow#1830)

    * Adding a sample for serving component

    * removed typo / updated based on PR feedback

    * fixing the jupyter rendering issue

    * adding pip3 for tensorflow

    * Fixed spelling error in VERSION

    * fix indentation based on review feedback

commit 54ff3e6
Author: Alexey Volkov <avolkov@google.com>
Date:   Fri Aug 16 01:22:31 2019 -0700

     SDK - Cleanup - Serialized PipelineParamTuple does not need value or type (kubeflow#1469)

    * SDK - Refactoring - Serialized PipelineParam does not need type
    Only the types in non-serialized PipelineParams are ever used.

    * SDK - Refactoring - Serialized PipelineParam does not need value
    Default values are only relevant when PipelineParam is used in the pipeline function signature and even in this case compiler captures them explicitly from the pipelineParam objects in the signature.
    There is no other uses for them.

commit d2e94e4
Author: Riley Bauer <34456002+rileyjbauer@users.noreply.github.com>
Date:   Thu Aug 15 19:52:34 2019 -0700

    Fix run duration bug (kubeflow#1827)

    * Allows durations >=24h and renames 'showLink' in RunList

    * Update, fix tests

commit d8eaeaa
Author: Alexey Volkov <avolkov@google.com>
Date:   Thu Aug 15 17:25:59 2019 -0700

    SDK - Preserving the pipeline input information in the compiled Workflow (kubeflow#1381)

    * SDK - Preserving the pipeline metadata in the compiled Workflow

    * Stabilizing the DSL compiler tests

commit afe8a69
Author: Kirin Patel <kirinpatel@gmail.com>
Date:   Thu Aug 15 17:00:28 2019 -0700

    Reduce API usage by utilizing reference name in reference resource API (kubeflow#1824)

    * Regenerated run api for frontend

    * Added support for reference name to resource reference API in frontend

    * Revert "Regenerated run api for frontend"

    * Addressed PR comments

    * Removed extra if statement by setting default value of parameter

    * Removed the whole comment

    * Addressed PR feedback

    * Addressed PR feedback

    * Simplified logic after offline discussion

commit ea67c99
Author: Kirin Patel <kirinpatel@gmail.com>
Date:   Thu Aug 15 16:18:35 2019 -0700

    Change how Variables are Provided to Visualizations (kubeflow#1754)

    * Changed way visualization variables are passed from request to NotebookNode

    Visualization variables are now saved to a json file and loaded by a NotebookNode upon execution.

    * Updated roc_curve visualization to reflect changes made to dependency injection

    * Fixed bug where checking if is_generated is provided to roc_curve visualization would crash visualizaiton

    Also changed ' -> "

    * Changed text_exporter to always sort variables by key for testing

    * Addressed PR suggestions

commit d238bef
Author: Jiaxiao Zheng <jxzheng@google.com>
Date:   Thu Aug 15 12:55:29 2019 -0700

    Add back coveralls. (kubeflow#1849)

    * Remove redundant import.

    * Simplify sample_test.yaml by using withItem syntax.

    * Simplify sample_test.yaml by using withItem syntax.

    * Change dict to str in withItems.

    * Add back coveralls.

commit 0517114
Author: Riley Bauer <34456002+rileyjbauer@users.noreply.github.com>
Date:   Thu Aug 15 12:28:35 2019 -0700

    Reduce getPipeline calls in RunList (kubeflow#1852)

    * Skips calling getPipeline in RunList if the pipeline name is in the pipeline_spec

    * Update fixed data to include pipeline names in pipeline specs

    * Remove redundant getRuns call

commit 39e5840
Author: IronPan <yangpa@google.com>
Date:   Thu Aug 15 11:04:34 2019 -0700

    Add retry button in Pipeline UI (kubeflow#1782)

    * add retry button

    * add retry button

    * add retry button

    * address comments

    * fix tests

    * fix tests

    * update image

    * Update StatusUtils.test.tsx

    * Update RunDetails.test.tsx

    * Update Buttons.ts

    * update test

    * update frontend

    * update

    * update

    * addrerss comments

    * update test
@gaoning777
Copy link
Contributor

#898 also adds the coverage. Which one do we want to merge?

@numerology
Copy link
Author

#898 also adds the coverage. Which one do we want to merge?

@gaoning777 Let's merge that one. Ark-kun also makes coverage display works IIRC.

@numerology
Copy link
Author

/close

@k8s-ci-robot
Copy link
Contributor

@numerology: Closed this PR.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@numerology numerology deleted the add-sdk-test-coverage branch August 26, 2019 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gaoning777 gaoning777 Awaiting requested review from gaoning777

@neuromage neuromage Awaiting requested review from neuromage

@Ark-kun Ark-kun Awaiting requested review from Ark-kun

Assignees
No one assigned
Labels
size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@numerology @k8s-ci-robot @Ark-kun @gaoning777