chore: modify test package name #6472

l-qing · 2023-03-31T07:50:22Z

Changes

chore: modify test package name
- Try to ensure that only exported functions are tested. I made this mistake in the previous PR.

Ref: https://github.com/tektoncd/community/blob/main/standards.md#go-packages

All exported functions should have tests
  * If your package is named "foo", prefer putting tests in a "foo_test" package in the same folder to ensure that only exported functions are tested

The shell command to search for test package names without the _test suffix is:

find . -name "*_test.go" | xargs grep -E '^package ' | grep -Ev '_test$' | grep -v '^./test' | sort

I did not finish all the changes this time because some packages do use private methods or variables.
I hope that we can follow the community's conventions as closely as possible, and provide a reference for the people who submit pull requests in the future.

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs included if any changes are user facing
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including
functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

tekton-robot · 2023-03-31T07:50:31Z

Hi @l-qing. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

l-qing · 2023-03-31T07:51:21Z

/kind misc

pkg/reconciler/pipelinerun/resources/resultrefresolution.go

tekton-robot · 2023-03-31T08:15:03Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/resolution/v1beta1/resolution_request_conversion.go	66.7%	0.0%	-66.7
pkg/reconciler/pipelinerun/resources/resultrefresolution.go	92.9%	89.0%	-3.9
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

l-qing · 2023-03-31T08:20:24Z

/retest

tekton-robot · 2023-03-31T08:20:39Z

@l-qing: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

tekton-robot · 2023-03-31T08:58:40Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

tekton-robot · 2023-04-01T00:28:31Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

tekton-robot · 2023-04-03T07:45:56Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

l-qing · 2023-04-03T15:05:44Z

/assign @lbernick

QuanZhang-William

Thanks @l-qing!

Looks like dot import is generally not recommended but useful golang tests. But anyways, the foo_test package can be used as a reference for people submitting PR in the future for sure👍 .

Related: #5111

vdemeester · 2023-04-04T13:49:41Z

Looks like dot import is the suggested method for golang tests. I agree this PR can be used as a reference for people submitting PR in the future .

😅

vdemeester · 2023-04-04T15:56:43Z

/ok-to-test

tekton-robot · 2023-04-04T16:02:05Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

vdemeester · 2023-04-04T16:05:21Z

So the use of dot-import here is to not have to prefix all types with v1 right ? The doc use case is more about circular dependencies, but I see it relatively useful here as well (to not have to rewrite all Pipeline to v1.Pipeline, etc..)

The import . form can be useful in tests that, due to circular dependencies, cannot be made part of the package being tested.

l-qing · 2023-04-11T02:11:35Z

/retest

vdemeester

Just one nit (a typo), otherwise LGTM

vdemeester · 2023-04-11T09:46:42Z

pkg/reconciler/pipelinerun/resources/validate_dependencies_test.go


 import (
 	"strings"
 	"testing"

 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
+	prresoruces "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources"


nit: this looks like a typo ? prresoruces -> prresources

Thanks, I have made the adjustments and rebased onto the latest main branch.

tekton-robot · 2023-04-11T13:00:06Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/resolution/v1alpha1/resolution_request_conversion.go	92.9%	53.6%	-39.3
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

tekton-robot · 2023-04-11T13:01:57Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/resolution/v1alpha1/resolution_request_conversion.go	92.9%	53.6%	-39.3
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

l-qing · 2023-04-11T16:25:50Z

/cc @vdemeester @jerop

tekton-robot · 2023-04-11T23:49:18Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/resolution/v1alpha1/resolution_request_conversion.go	92.9%	53.6%	-39.3
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

tekton-robot · 2023-04-11T23:50:28Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/resolution/v1alpha1/resolution_request_conversion.go	92.9%	53.6%	-39.3
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

tekton-robot · 2023-04-13T07:16:14Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/resolution/v1alpha1/resolution_request_conversion.go	92.9%	53.6%	-39.3
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

tekton-robot · 2023-04-13T07:26:05Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/resolution/v1alpha1/resolution_request_conversion.go	92.9%	53.6%	-39.3
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

l-qing · 2023-04-13T08:34:51Z

/retest

tekton-robot · 2023-04-15T02:49:00Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/resolution/v1alpha1/resolution_request_conversion.go	92.9%	53.6%	-39.3
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

tekton-robot · 2023-04-15T02:50:39Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/resolution/v1alpha1/resolution_request_conversion.go	92.9%	53.6%	-39.3
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

Try to ensure that only exported functions are tested. Ref: https://github.com/tektoncd/community/blob/main/standards.md#go-packages ``` All exported functions should have tests * If your package is named "foo", prefer putting tests in a "foo_test" package in the same folder to ensure that only exported functions are tested ``` The shell command to search for test package names without the `_test` suffix is: * `find . -name "*_test.go" | xargs grep -E '^package ' | grep -Ev '_test$' | grep -v '^./test' | sort`

tekton-robot · 2023-04-17T13:26:21Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/resolution/v1alpha1/resolution_request_conversion.go	92.9%	53.6%	-39.3
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

tekton-robot · 2023-04-17T13:26:29Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/resolution/v1alpha1/resolution_request_conversion.go	92.9%	53.6%	-39.3
pkg/resolution/resolver/framework/configstore.go	26.1%	47.8%	21.7

lbernick · 2023-04-18T13:18:41Z

looks like Jerop and Vincent's feedback has been addressed.
/lgtm

l-qing · 2023-04-18T13:57:37Z

looks like Jerop and Vincent's feedback has been addressed.

Thank you for your comment.

tekton-robot added the release-note-none Denotes a PR that doesnt merit a release note. label Mar 31, 2023

tekton-robot requested review from dibyom and imjasonh March 31, 2023 07:50

tekton-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 31, 2023

tekton-robot added the kind/misc Categorizes issue or PR as a miscellaneuous one. label Mar 31, 2023

l-qing commented Mar 31, 2023

View reviewed changes

pkg/reconciler/pipelinerun/resources/resultrefresolution.go Outdated Show resolved Hide resolved

l-qing force-pushed the chore/modify-test-package-name branch from b7c5416 to 631991e Compare March 31, 2023 08:45

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 31, 2023

l-qing force-pushed the chore/modify-test-package-name branch from 631991e to 230e5dd Compare April 1, 2023 00:19

l-qing force-pushed the chore/modify-test-package-name branch from 230e5dd to 3d8892b Compare April 3, 2023 07:25

tekton-robot assigned lbernick Apr 3, 2023

QuanZhang-William reviewed Apr 4, 2023

View reviewed changes

tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 4, 2023

l-qing force-pushed the chore/modify-test-package-name branch from 3d8892b to 0ab31a1 Compare April 4, 2023 16:19

vdemeester reviewed Apr 11, 2023

View reviewed changes

l-qing force-pushed the chore/modify-test-package-name branch from 8b2a1eb to 2faf8ee Compare April 11, 2023 12:54

tekton-robot requested review from jerop and vdemeester April 11, 2023 16:25

l-qing force-pushed the chore/modify-test-package-name branch from 2faf8ee to 3f4ada2 Compare April 11, 2023 23:43

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2023

l-qing force-pushed the chore/modify-test-package-name branch from 3f4ada2 to 34a67df Compare April 13, 2023 07:10

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 13, 2023

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2023

l-qing force-pushed the chore/modify-test-package-name branch from 34a67df to 7515c37 Compare April 15, 2023 02:43

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2023

l-qing force-pushed the chore/modify-test-package-name branch from 7515c37 to 3b0f7bd Compare April 17, 2023 13:19

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 18, 2023

tekton-robot merged commit 9b30f08 into tektoncd:main Apr 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: modify test package name #6472

chore: modify test package name #6472

l-qing commented Mar 31, 2023 •

edited

Loading

tekton-robot commented Mar 31, 2023

l-qing commented Mar 31, 2023

tekton-robot commented Mar 31, 2023

l-qing commented Mar 31, 2023

tekton-robot commented Mar 31, 2023

tekton-robot commented Mar 31, 2023

tekton-robot commented Apr 1, 2023

tekton-robot commented Apr 3, 2023

l-qing commented Apr 3, 2023

QuanZhang-William left a comment •

edited

Loading

vdemeester commented Apr 4, 2023

vdemeester commented Apr 4, 2023

tekton-robot commented Apr 4, 2023

vdemeester commented Apr 4, 2023

l-qing commented Apr 11, 2023

vdemeester left a comment

vdemeester Apr 11, 2023

l-qing Apr 11, 2023

tekton-robot commented Apr 11, 2023

tekton-robot commented Apr 11, 2023

l-qing commented Apr 11, 2023

tekton-robot commented Apr 11, 2023

tekton-robot commented Apr 11, 2023

tekton-robot commented Apr 13, 2023

tekton-robot commented Apr 13, 2023

l-qing commented Apr 13, 2023

tekton-robot commented Apr 15, 2023

tekton-robot commented Apr 15, 2023

tekton-robot commented Apr 17, 2023

tekton-robot commented Apr 17, 2023

lbernick commented Apr 18, 2023

l-qing commented Apr 18, 2023

chore: modify test package name #6472

chore: modify test package name #6472

Conversation

l-qing commented Mar 31, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Mar 31, 2023

l-qing commented Mar 31, 2023

tekton-robot commented Mar 31, 2023

l-qing commented Mar 31, 2023

tekton-robot commented Mar 31, 2023

tekton-robot commented Mar 31, 2023

tekton-robot commented Apr 1, 2023

tekton-robot commented Apr 3, 2023

l-qing commented Apr 3, 2023

QuanZhang-William left a comment • edited Loading

Choose a reason for hiding this comment

vdemeester commented Apr 4, 2023

vdemeester commented Apr 4, 2023

tekton-robot commented Apr 4, 2023

vdemeester commented Apr 4, 2023

l-qing commented Apr 11, 2023

vdemeester left a comment

Choose a reason for hiding this comment

vdemeester Apr 11, 2023

Choose a reason for hiding this comment

l-qing Apr 11, 2023

Choose a reason for hiding this comment

tekton-robot commented Apr 11, 2023

tekton-robot commented Apr 11, 2023

l-qing commented Apr 11, 2023

tekton-robot commented Apr 11, 2023

tekton-robot commented Apr 11, 2023

tekton-robot commented Apr 13, 2023

tekton-robot commented Apr 13, 2023

l-qing commented Apr 13, 2023

tekton-robot commented Apr 15, 2023

tekton-robot commented Apr 15, 2023

tekton-robot commented Apr 17, 2023

tekton-robot commented Apr 17, 2023

lbernick commented Apr 18, 2023

l-qing commented Apr 18, 2023

l-qing commented Mar 31, 2023 •

edited

Loading

QuanZhang-William left a comment •

edited

Loading