Migrate upgrade tests to the new framework by mgencur · Pull Request #10216 · knative/serving

mgencur · 2020-11-27T12:21:33Z

Proposed Changes

Use the upgrade framework from knative.dev/pkg/test/upgrade
Convert AssertAutoscaleUpToNumPods and inner functions to helper functions that return error instead of calling t.Fatal to fail the current test. Avoid using *testing.T so that these functions can be reused outside of tests or span multiple tests.
The autoscaler helper functions are used in upgrade tests where "setup" and "verify" phases run within different tests. Pull test.EnsureTearDown from SetupSvc to ensure that a kservice is not destroyed at the end of the first phase ("setup") but remains active until "verify" phase. This is ensured by calling EnsureTearDown later in the "verify" phase.
Adjust Bash scripts to avoid unbound variable errors during upgrade tests exexution (due to using -u flag by the upgrade framework).

Release Note

codecov · 2020-11-27T12:27:03Z

Codecov Report

Merging #10216 (5dbd73b) into master (90291ee) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #10216      +/-   ##
==========================================
+ Coverage   87.99%   88.01%   +0.01%     
==========================================
  Files         187      187              
  Lines        8808     8808              
==========================================
+ Hits         7751     7752       +1     
- Misses        815      816       +1     
+ Partials      242      240       -2

Impacted Files	Coverage Δ
pkg/autoscaler/scaling/multiscaler.go	`87.71% <0.00%> (-1.76%)`	⬇️
pkg/reconciler/configuration/configuration.go	`86.71% <0.00%> (-1.57%)`	⬇️
pkg/activator/net/revision_backends.go	`90.41% <0.00%> (-0.92%)`	⬇️
pkg/reconciler/autoscaling/kpa/scaler.go	`90.00% <0.00%> (+1.42%)`	⬆️
pkg/autoscaler/statforwarder/forwarder.go	`96.29% <0.00%> (+5.55%)`	⬆️
pkg/autoscaler/statforwarder/processor.go	`88.88% <0.00%> (+5.55%)`	⬆️

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 90291ee...429b845. Read the comment docs.

cardil

Looks quite well. I found some minor issues.

mgencur · 2020-12-01T15:27:12Z

/retest

mgencur · 2020-12-01T15:47:44Z

/retest

mgencur · 2020-12-02T10:13:49Z

/retest

mgencur · 2020-12-02T11:10:54Z

/retest

mgencur · 2020-12-02T12:55:43Z

The error in pull-knative-serving-istio-stable-no-mesh-tls doesn't seem to be related to this PR:

2020/12/02 11:33:53 Failed to setup DNS record: timed out waiting for the condition

* Convert AssertAutoscaleUpToNumPods and inner functions to helper functions that return error instead of calling t.Fatal to fail the current test. Avoid using *testing.T so that these functions can be reused outside of tests or span multiple tests. * The autoscaler helper functions are used in upgrade tests where "setup" and "verify" phases run within different tests. Pull test.EnsureTearDown from SetupSvc to ensure that a kservice is not destroyed at the end of the first phase ("setup") but remains active until "verify" phase. This is ensured by calling EnsureTearDown later in the "verify" phase. * Adjust Bash scripts to avoid unbound variable errors during upgrade tests exexution (due to using -u flag by the upgrade framework).

That's for easy reuse in other repos such as knative/operator

* autoscale and prober tests will fail if ingress is replaced during upgrade

nak3 · 2021-01-05T07:25:18Z

/lgtm
/approve

knative-prow-robot · 2021-01-05T07:25:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil, mgencur, nak3

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [nak3]

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

mgencur · 2021-01-05T07:26:14Z

/hold cancel

I believe the hold label from Markus is redundant at this point. The upgrade tests were fixed some time ago.

mgencur · 2021-01-05T08:12:46Z

/retest

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 27, 2020

google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label Nov 27, 2020

knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/test-and-release It flags unit/e2e/conformance/perf test issues for product features labels Nov 27, 2020

knative-prow-robot requested review from chaodaiG and coryrc November 27, 2020 12:21

mgencur force-pushed the migrate_upgrade_tests branch 8 times, most recently from a1ff592 to d33a694 Compare November 30, 2020 15:48

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 1, 2020

mgencur force-pushed the migrate_upgrade_tests branch from f81894e to 4e5454a Compare December 1, 2020 07:10

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 1, 2020

mgencur force-pushed the migrate_upgrade_tests branch 2 times, most recently from cdbb0a4 to 28d884c Compare December 1, 2020 07:51

mgencur changed the title ~~[WIP] Migrate upgrade tests to the new framework~~ Migrate upgrade tests to the new framework Dec 1, 2020

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 1, 2020

cardil requested changes Dec 1, 2020

View reviewed changes

mgencur force-pushed the migrate_upgrade_tests branch 2 times, most recently from 162bead to cb68163 Compare December 2, 2020 09:36

knative-prow-robot assigned mattmoor Jan 4, 2021

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 4, 2021

mgencur added 18 commits January 5, 2021 08:12

Use umbrella functions for individual groups of tests

0c652e1

That's for easy reuse in other repos such as knative/operator

Define curPods and targetPods constants in upgrade tests

b08399a

Fix imports and licences

27f5b04

Fix lint - comments on new exported functions

63a05db

Split AssertAutoscaleUpToNumPods into setup and wait parts

272a420

Pass errgroup by reference

9ef05fc

Fix lint error

150bac8

Move logf into TestContext

75acab5

Remaining fixes for moving logf to TestContext

d0d348a

Call logf on context directly

7dc161c

Update readme

35ce9f4

Remove the comment and fix imports

7b60231

Update modules after running update-codegen

6029f1c

Do not upgrade Ingress when upgrading Knative

0449e67

* autoscale and prober tests will fail if ingress is replaced during upgrade

Pass around func() error instead of errGroup

a7ed4fc

Mark a few functions as t.Helper()

1ff2fbe

Fix codegen

429b845

mgencur force-pushed the migrate_upgrade_tests branch from f6b2ac7 to 429b845 Compare January 5, 2021 07:12

knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 5, 2021

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 5, 2021

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 5, 2021

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 5, 2021

knative-prow-robot merged commit d2f2944 into knative:master Jan 5, 2021

Conversation

mgencur commented Nov 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Uh oh!

codecov Bot commented Nov 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cardil left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mgencur commented Dec 1, 2020

Uh oh!

mgencur commented Dec 1, 2020

Uh oh!

mgencur commented Dec 2, 2020

Uh oh!

mgencur commented Dec 2, 2020

Uh oh!

mgencur commented Dec 2, 2020

Uh oh!

nak3 commented Jan 5, 2021

Uh oh!

knative-prow-robot commented Jan 5, 2021

Uh oh!

mgencur commented Jan 5, 2021

Uh oh!

mgencur commented Jan 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mgencur commented Nov 27, 2020 •

edited

Loading

codecov Bot commented Nov 27, 2020 •

edited

Loading