Skip to content

Commit

Permalink
review: address comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Daniel Hiller <[email protected]>
  • Loading branch information
@dhiller
dhiller committed Dec 4, 2023
1 parent 08d8f6b commit 85c48f9
Show file tree
Hide file tree
Showing 11 changed files with 95 additions and 133 deletions.
7 changes: 0 additions & 7 deletions WORKSPACE
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,6 @@ container_pull(
tag = "v20210120-b86882c9314933ba1a0c77965ed9d54a747f7957",
)

container_pull(
name = "botreview-base",
registry = "index.docker.io",
repository = "alpine/git",
tag = "v2.40.1",
)

load(
"@io_bazel_rules_docker//go:image.bzl",
_go_image_repos = "repositories",
Expand Down
3 changes: 1 addition & 2 deletions external-plugins/botreview/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,4 @@ test:
$(bazelbin) test //external-plugins/botreview/...

push:
$(bazelbin) run --stamp --workspace_status_command="./hack/print-workspace-status-no-git-tag.sh" //external-plugins/botreview:push
bash -x ../../hack/update-deployments-with-latest-image.sh quay.io/kubevirtci/botreview
podman build -f ./images/botreview/Containerfile -t quay.io/kubevirtci/botreview:latest && podman push quay.io/kubevirtci/botreview:latest
2 changes: 1 addition & 1 deletion external-plugins/botreview/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ Updates in these PRs are mostly tedious to review for a human, since they contai

What `botreview` can at least do is automate what a human would do anyway, like applying an expected change pattern to the changes. And this is what botreview does.

`botreview` has of course room for improval, i.e. it might generate a list of the images and check whether these are pull-able, or even perform further checks on the images. **Note: the latter is not implemented (yet)**
`botreview` has of course room for improvement, i.e. it might generate a list of the images and check whether these are pullable, or even perform further checks on the images. **Note: the latter is not implemented (yet)**
44 changes: 22 additions & 22 deletions external-plugins/botreview/review/bump_kubevirtci_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,26 @@ import (
)

func TestBumpKubevirtCI_Review(t1 *testing.T) {
diffFilePathes := []string{}
diffFilePaths := []string{}
entries, err := os.ReadDir("testdata/kubevirtci-bump")
if err != nil {
t1.Errorf("failed to read files: %v", err)
}
for _, entry := range entries {
diffFilePathes = append(diffFilePathes, filepath.Join("testdata/kubevirtci-bump", entry.Name()))
diffFilePaths = append(diffFilePaths, filepath.Join("testdata/kubevirtci-bump", entry.Name()))
}
diffFilePathes = append(diffFilePathes, "testdata/mixed_bump_prow_job.patch0")
diffFilePathesToDiffs := map[string]*diff.FileDiff{}
for _, diffFile := range diffFilePathes {
bump_images_diff_file, err := os.ReadFile(diffFile)
diffFilePaths = append(diffFilePaths, "testdata/mixed_bump_prow_job.patch0")
diffFilePathsToDiffs := map[string]*diff.FileDiff{}
for _, diffFile := range diffFilePaths {
bumpImagesDiffFile, err := os.ReadFile(diffFile)
if err != nil {
t1.Errorf("failed to read diff: %v", err)
}
bump_file_diffs, err := diff.ParseFileDiff(bump_images_diff_file)
bumpFileDiffs, err := diff.ParseFileDiff(bumpImagesDiffFile)
if err != nil {
t1.Errorf("failed to read diff: %v", err)
}
diffFilePathesToDiffs[diffFile] = bump_file_diffs
diffFilePathsToDiffs[diffFile] = bumpFileDiffs
}
type fields struct {
relevantFileDiffs []*diff.FileDiff
Expand All @@ -61,14 +61,14 @@ func TestBumpKubevirtCI_Review(t1 *testing.T) {
name: "simple prow autobump",
fields: fields{
relevantFileDiffs: []*diff.FileDiff{
diffFilePathesToDiffs["testdata/kubevirtci-bump/cluster-up-sha.txt"],
diffFilePathesToDiffs["testdata/kubevirtci-bump/cluster-up_cluster_kind-1.22-sriov_provider.sh"],
diffFilePathesToDiffs["testdata/kubevirtci-bump/cluster-up_cluster_kind-1.22-sriov_sriov-node_node.sh"],
diffFilePathesToDiffs["testdata/kubevirtci-bump/cluster-up_cluster_kind_common.sh"],
diffFilePathesToDiffs["testdata/kubevirtci-bump/cluster-up_cluster_kind_configure-registry-proxy.sh"],
diffFilePathesToDiffs["testdata/kubevirtci-bump/cluster-up_hack_common.sh"],
diffFilePathesToDiffs["testdata/kubevirtci-bump/cluster-up_version.txt"],
diffFilePathesToDiffs["testdata/kubevirtci-bump/hack_config-default.sh"],
diffFilePathsToDiffs["testdata/kubevirtci-bump/cluster-up-sha.txt"],
diffFilePathsToDiffs["testdata/kubevirtci-bump/cluster-up_cluster_kind-1.22-sriov_provider.sh"],
diffFilePathsToDiffs["testdata/kubevirtci-bump/cluster-up_cluster_kind-1.22-sriov_sriov-node_node.sh"],
diffFilePathsToDiffs["testdata/kubevirtci-bump/cluster-up_cluster_kind_common.sh"],
diffFilePathsToDiffs["testdata/kubevirtci-bump/cluster-up_cluster_kind_configure-registry-proxy.sh"],
diffFilePathsToDiffs["testdata/kubevirtci-bump/cluster-up_hack_common.sh"],
diffFilePathsToDiffs["testdata/kubevirtci-bump/cluster-up_version.txt"],
diffFilePathsToDiffs["testdata/kubevirtci-bump/hack_config-default.sh"],
},
},
want: &BumpKubevirtCIResult{},
Expand All @@ -77,21 +77,21 @@ func TestBumpKubevirtCI_Review(t1 *testing.T) {
name: "mixed image bump",
fields: fields{
relevantFileDiffs: []*diff.FileDiff{
diffFilePathesToDiffs["testdata/kubevirtci-bump/cluster-up-sha.txt"],
diffFilePathesToDiffs["testdata/kubevirtci-bump/cluster-up_cluster_kind-1.22-sriov_provider.sh"],
diffFilePathesToDiffs["testdata/mixed_bump_prow_job.patch0"],
diffFilePathsToDiffs["testdata/kubevirtci-bump/cluster-up-sha.txt"],
diffFilePathsToDiffs["testdata/kubevirtci-bump/cluster-up_cluster_kind-1.22-sriov_provider.sh"],
diffFilePathsToDiffs["testdata/mixed_bump_prow_job.patch0"],
},
},
want: &BumpKubevirtCIResult{
notMatchingHunks: map[string][]*diff.Hunk{"github/ci/prow-deploy/files/jobs/kubevirt/kubevirt/kubevirt-presubmits.yaml": diffFilePathesToDiffs["testdata/mixed_bump_prow_job.patch0"].Hunks},
notMatchingHunks: map[string][]*diff.Hunk{"github/ci/prow-deploy/files/jobs/kubevirt/kubevirt/kubevirt-presubmits.yaml": diffFilePathsToDiffs["testdata/mixed_bump_prow_job.patch0"].Hunks},
},
},
}
for _, tt := range tests {
t1.Run(tt.name, func(t1 *testing.T) {
t := &BumpKubevirtCI{}
for _, diff := range tt.fields.relevantFileDiffs {
t.AddIfRelevant(diff)
for _, fileDiff := range tt.fields.relevantFileDiffs {
t.AddIfRelevant(fileDiff)
}
if got := t.Review(); !reflect.DeepEqual(got, tt.want) {
t1.Errorf("Review() = %v, want %v", got, tt.want)
Expand Down
20 changes: 10 additions & 10 deletions external-plugins/botreview/review/image_update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,22 @@ import (
)

func TestProwJobImageUpdate_Review(t1 *testing.T) {
diffFilePathes := []string{
diffFilePaths := []string{
"testdata/simple_bump-prow-job-images_sh.patch0",
"testdata/simple_bump-prow-job-images_sh.patch1",
"testdata/mixed_bump_prow_job.patch0",
}
diffFilePathesToDiffs := map[string]*diff.FileDiff{}
for _, diffFile := range diffFilePathes {
bump_images_diff_file, err := os.ReadFile(diffFile)
diffFilePathsToDiffs := map[string]*diff.FileDiff{}
for _, diffFile := range diffFilePaths {
bumpImagesDiffFile, err := os.ReadFile(diffFile)
if err != nil {
t1.Errorf("failed to read diff: %v", err)
}
bump_file_diffs, err := diff.ParseFileDiff(bump_images_diff_file)
bumpFileDiffs, err := diff.ParseFileDiff(bumpImagesDiffFile)
if err != nil {
t1.Errorf("failed to read diff: %v", err)
}
diffFilePathesToDiffs[diffFile] = bump_file_diffs
diffFilePathsToDiffs[diffFile] = bumpFileDiffs
}
type fields struct {
relevantFileDiffs []*diff.FileDiff
Expand All @@ -56,8 +56,8 @@ func TestProwJobImageUpdate_Review(t1 *testing.T) {
name: "simple image bump",
fields: fields{
relevantFileDiffs: []*diff.FileDiff{
diffFilePathesToDiffs["testdata/simple_bump-prow-job-images_sh.patch0"],
diffFilePathesToDiffs["testdata/simple_bump-prow-job-images_sh.patch1"],
diffFilePathsToDiffs["testdata/simple_bump-prow-job-images_sh.patch0"],
diffFilePathsToDiffs["testdata/simple_bump-prow-job-images_sh.patch1"],
},
},
want: &ProwJobImageUpdateResult{},
Expand All @@ -66,11 +66,11 @@ func TestProwJobImageUpdate_Review(t1 *testing.T) {
name: "mixed image bump",
fields: fields{
relevantFileDiffs: []*diff.FileDiff{
diffFilePathesToDiffs["testdata/mixed_bump_prow_job.patch0"],
diffFilePathsToDiffs["testdata/mixed_bump_prow_job.patch0"],
},
},
want: &ProwJobImageUpdateResult{
notMatchingHunks: map[string][]*diff.Hunk{"github/ci/prow-deploy/files/jobs/kubevirt/kubevirt/kubevirt-presubmits.yaml": {diffFilePathesToDiffs["testdata/mixed_bump_prow_job.patch0"].Hunks[0]}},
notMatchingHunks: map[string][]*diff.Hunk{"github/ci/prow-deploy/files/jobs/kubevirt/kubevirt/kubevirt-presubmits.yaml": {diffFilePathsToDiffs["testdata/mixed_bump_prow_job.patch0"].Hunks[0]}},
},
},
}
Expand Down
82 changes: 41 additions & 41 deletions external-plugins/botreview/review/prow_autobump_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,25 @@ import (
)

func TestProwAutobump_Review(t1 *testing.T) {
diffFilePathes := []string{}
diffFilePaths := []string{}
entries, err := os.ReadDir("testdata/prow-autobump")
if err != nil {
t1.Errorf("failed to read files: %v", err)
}
for _, entry := range entries {
diffFilePathes = append(diffFilePathes, filepath.Join("testdata/prow-autobump", entry.Name()))
diffFilePaths = append(diffFilePaths, filepath.Join("testdata/prow-autobump", entry.Name()))
}
diffFilePathesToDiffs := map[string]*diff.FileDiff{}
for _, diffFile := range diffFilePathes {
bump_images_diff_file, err := os.ReadFile(diffFile)
diffFilePathsToDiffs := map[string]*diff.FileDiff{}
for _, diffFile := range diffFilePaths {
bumpImagesDiffFile, err := os.ReadFile(diffFile)
if err != nil {
t1.Errorf("failed to read diff: %v", err)
}
bump_file_diffs, err := diff.ParseFileDiff(bump_images_diff_file)
bumpFileDiffs, err := diff.ParseFileDiff(bumpImagesDiffFile)
if err != nil {
t1.Errorf("failed to read diff: %v", err)
}
diffFilePathesToDiffs[diffFile] = bump_file_diffs
diffFilePathsToDiffs[diffFile] = bumpFileDiffs
}
type fields struct {
relevantFileDiffs []*diff.FileDiff
Expand All @@ -60,22 +60,22 @@ func TestProwAutobump_Review(t1 *testing.T) {
name: "simple prow autobump",
fields: fields{
relevantFileDiffs: []*diff.FileDiff{
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_configs_current_config_config.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_branch-protector.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_cherrypicker_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_label-sync-kubevirt.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_label-sync-nmstate.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_crier_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_deck_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_ghproxy.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_hook_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_horologium_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_needs-rebase_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_prow_controller_manager_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_sinker_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_statusreconciler_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_tide_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_overlays_ibmcloud-production_resources_prow-exporter-deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_configs_current_config_config.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_branch-protector.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_cherrypicker_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_label-sync-kubevirt.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_label-sync-nmstate.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_crier_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_deck_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_ghproxy.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_hook_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_horologium_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_needs-rebase_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_prow_controller_manager_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_sinker_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_statusreconciler_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_tide_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_overlays_ibmcloud-production_resources_prow-exporter-deployment.yaml"],
},
},
want: &ProwAutobumpResult{},
Expand All @@ -84,28 +84,28 @@ func TestProwAutobump_Review(t1 *testing.T) {
name: "prow autobump with crd update",
fields: fields{
relevantFileDiffs: []*diff.FileDiff{
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_configs_current_config_config.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_branch-protector.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_cherrypicker_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_label-sync-kubevirt.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_label-sync-nmstate.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_crier_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_deck_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_ghproxy.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_hook_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_horologium_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_needs-rebase_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_prow_controller_manager_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_prowjob-crd_prowjob_customresourcedefinition.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_sinker_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_statusreconciler_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_tide_deployment.yaml"],
diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_overlays_ibmcloud-production_resources_prow-exporter-deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_configs_current_config_config.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_branch-protector.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_cherrypicker_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_label-sync-kubevirt.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_local_label-sync-nmstate.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_crier_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_deck_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_ghproxy.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_hook_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_horologium_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_needs-rebase_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_prow_controller_manager_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_prowjob-crd_prowjob_customresourcedefinition.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_sinker_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_statusreconciler_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_tide_deployment.yaml"],
diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_overlays_ibmcloud-production_resources_prow-exporter-deployment.yaml"],
},
},
want: &ProwAutobumpResult{
notMatchingHunks: map[string][]*diff.Hunk{
"github/ci/prow-deploy/kustom/base/manifests/test_infra/current/prowjob-crd/prowjob_customresourcedefinition.yaml": diffFilePathesToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_prowjob-crd_prowjob_customresourcedefinition.yaml"].Hunks,
"github/ci/prow-deploy/kustom/base/manifests/test_infra/current/prowjob-crd/prowjob_customresourcedefinition.yaml": diffFilePathsToDiffs["testdata/prow-autobump/github_ci_prow-deploy_kustom_base_manifests_test_infra_current_prowjob-crd_prowjob_customresourcedefinition.yaml"].Hunks,
},
},
},
Expand Down
Loading

0 comments on commit 85c48f9

Please sign in to comment.