Fix: don't fail test helpers if cleanup returns a 404

[sebasslash]

Description

We've had a series of flaky tests where the test itself succeeds but the cleanup fails, most notably workspaces deleting their children before the test is able to make the API call to delete a given child resource. We want to reduce the possibility of a flake during cleanup.

The idea here is we shouldn't fail a test if the resource that it's trying to clean up is not found (ErrResourceNotFound). We don't use explicit synchronization when firing off API calls which could explain why parent resources are occasionally deleted first. Rather than to wrangle with explicit ordering, we can simply add a fail-safe to ignore deleting non existent resources.

It is worth mentioning, all remaining resources are destroyed when the tflocal instance is rebuilt nightly so these dangling resources don't sit around for too long. We don't however ignore all deletion errors since they can potentially raise an API level concern.

External links

• Comment that posed question

[brandonc]

I like that this targets 404 errors only, and I only have one reservation: 404 is also used when an authorization error occurs so that could potentially hide a bug that the test runner does not have authorization to delete something, which would be surprising.

I think I am leaning toward being against this change because proper ordering (deleting the thing the workspace owns before deleting the workspace, for instance) and using t.Cleanup so ordering is simplified eliminates the problem. I don't think this has the potential to resolve flakes that are otherwise successful tests, but I think it has the potential to obscure test structure bugs or authorization bugs.

[Uk1288]

Good point, sounds like we might have to handle this on a case by case flake. Given that some of the clean up failures are difficult to reproduce, I am thinking we should monitor and fix them as they occur.

[sebasslash]

Hmm, I don't think this obscures an authorization bug because we always test the delete method for a given interface -- which if in the case it was an auth bug, those tests would fail as well. I think if we're getting a 404 during the cleanup, but the related delete integration test succeeds, it is an actual 404.

[laurenolivia]

@sebasslash The StateVersion flake failing your PR has been fixed 😸

[sebasslash]

Alrighty, there haven't been any more comments and it seems we're set on failing tests with 404s on cleanup. Closing.