Re-enable TestCronVulnerabilitiesCreatesDatabasesPath test with more precise SUT and internal panic recovery (#23468)

#23258 (see
[comment](https://github.com/fleetdm/fleet/issues/23258#issuecomment-2443304838)
for rationale)

Validated by removing the two places that would create the directory
(early in scanVulnerabilities in cron.go, partway through download in
download.go) and ensuring the test failed (timeout after 10s).

Both dir creations happen early in the vulns cron so I was able to
drastically tighten the timing on the periodic check on this test, so
this tests completes way quicker than before as an added benefit
(automatic test parallelism notwithstanding).

The panic recovery here theoretically shouldn't be necessary, as on a
passed test the context will get cancelled while syncing the CPE sqlite,
but is included to ensure the test doesn't flake if the implementation
of the vulnerabilities cron changes such that we _would_ get a panic by
cancelling the context this early.

# Checklist for submitter
- [x] Added/updated tests
This commit is contained in:
Ian Littman 2024-11-05 07:34:54 -06:00 committed by GitHub
parent c025d89f2f
commit a2468fa0b4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -307,7 +307,6 @@ func TestAutomationsSchedule(t *testing.T) {
}
func TestCronVulnerabilitiesCreatesDatabasesPath(t *testing.T) {
t.Skip() // https://github.com/fleetdm/fleet/issues/23258
t.Parallel()
ctx, cancelFunc := context.WithCancel(context.Background())
defer cancelFunc()
@ -355,24 +354,26 @@ func TestCronVulnerabilitiesCreatesDatabasesPath(t *testing.T) {
ctx = license.NewContext(ctx, &fleet.LicenseInfo{Tier: fleet.TierPremium})
ctx, cancel := context.WithCancel(ctx)
lg := kitlog.NewJSONLogger(os.Stdout)
s, err := newVulnerabilitiesSchedule(ctx, "test_instance", ds, lg, &config)
require.NoError(t, err)
s.Start()
t.Cleanup(func() {
cancel()
<-s.Done()
})
go func() {
defer func() {
// this test may panic as we're ending it early, so we shouldn't fail our suite on this panic
// but depending on where we are in the cron call it may not panic, hence not checking the recover
// value either way
// nolint:errcheck
recover()
}()
_ = cronVulnerabilities(ctx, ds, lg, &config)
}()
assert.Eventually(t, func() bool {
info, err := os.Lstat(vulnPath)
if err != nil {
return false
}
if !info.IsDir() {
return false
}
return true
}, 5*time.Minute, 30*time.Second)
return err == nil && info.IsDir()
}, 10*time.Second, 1*time.Second)
t.Cleanup(func() {
cancel()
})
// at this point, the assertion has succeeded or failed, try to remove the
// temp dir and all content instead of leaving it to the testing package to