Do not use -race flag in upgrade tests

[mgencur]

The test suite occasionally fails with a race condition in triggering
the continual tests. Disabling the -race flag should not have effect on
how individual tests run. The tests are re-used from other repositories.
When the issue with the upgrade framework is fixed this can be enabled
again.

/assign @cardil

[markusthoemmes]

Have we filed issues upstream to actually hunt and fix these bugs?

[mgencur]

Just filed one: knative/pkg#2026

[cardil]

The race flag doesn't make much sense in context of upgrade testing.

[markusthoemmes]

I disagree with that. If the framework itself has data races going on, we might have racy outcomes and don't know where they're coming from. That might result in false positives or false negatives which would be quite annoying.

[markusthoemmes]

(That said, this is LGTM since we've opened a tracking bug to actually fix it and bothering us with errors ain't useful)

[cardil]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mgencur]

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

[cardil]

I disagree with that. If the framework itself has data races going on, we might have racy outcomes and don't know where they're coming from. That might result in false positives or false negatives which would be quite annoying.

Only false positives, like this. Race recognition makes sense when unit testing. If you deploy prebuild artifacts like all e2e tests do, races will be reported only in testing/orchestration code. That code isn't run on production. That's why I think it makes a little sense to use -race for upgrade testing, as it has its downsides of longer operation and different output processing.

[markusthoemmes]

@cardil well but in this case the race is actually happening in the test code itself, which means that it has a bug that has to be fixed too. Taken to the extreme, such a bug could lead to a test passing when it actually should fail (I'm not saying that's the case here but still).