can we group the imports by

• go libs
• knative libs
• everything
  ?

Will align them in the correct order.

common.go is overly generic.
can we break it out to
verify.go - to cover all the assert/verify helpers
setup.go - to cover all the setup/creation helpers?

I will create a file called setup.go, and put it under the folder client.
The file verify.go can be put under the folder resources, since it verifies everything about the knative resources.

im conflicted about the naming
i understand the CRD is called KnativeServing, but a method that says KnativeServingVerify is confusing for me.
maybe append CR at the end, as we are verifying the CR status.
thoughts?

can we pass in the configmap to this helper.
that way it can be reused for other changes

debating if we need to prefix everything with KnativeServing

We can use KS for short.

would break this into separate methods
deleteDeployments
verifyDeployments.

you can keep this method, but rename it DeleteAndVerifyDeployments, to make it clear all that the method is doing.

same as above.
split this into smaller pieces.

is there a way to determine this dynamically? the version should be in a file in third_party/?

1.4-latest is taken as default, if this param is not set.
HOWEVER, the directory is not available under the path, https://raw.githubusercontent.com/knative/serving, but it is available under the source code folder.
It only happens to this 1.X-latest directory, not to others like 1.4.2.
I have to set a specific version here to access via https://raw.githubusercontent.com/knative/serving/\<version>

would prefer not to use the "common" folder name, as typically ends up the dumping grounds for everything.

why is this capitalized

can this ever be false? isn't it always set and can't be overwritten?

Will remove it.

any reason not to combine line 41 and 42 together?

local full_url="https://github.com/knative/serving-operator/releases/download/${LATEST_SERVING_RELEASE_VERSION}/serving-operator.yaml"

No, just legacy.

should the var be lower case?
im assuming all local var using lower case for consistency

Done.

If the method is fatal, consider renaming it to assert
or have it return an error.

please include Ready in the method name.

Done.

maybe rename this to EnsureKnativeServingExists?

Done.

how is this deployment test different than the post upgrade test?

The commonality is that at the beginning we need to create and make sure the operator CR is ready, and in the end remove the operator CR.

The difference is that integration tests tagged with e2e will run the verification of the knative serving deployments including configuration and recreation. The tests tagged with postupgrade will verify what should be ready after upgrading the operator from the previous release, like whether we have exactly the same number and names of deployments as expected(like in eventing operator, we have got a new deployment and removed an old one. The post upgrade tests there verify if we have the correct list of deployment). Later, I will move all the existing postupgrade tests available in knative serving repo into this post upgrade test in operator, so that we make operator the default and optimal tool to upgrade.

Next step, I will open PRs to port the post upgrade tests in serving here to operator.