Skip flaky TestActions on MacOSx

[ChrsMark]

What does this PR do?

This PR skips flaky test case reported at #23965.

[elasticmachine]

Pinging @elastic/siem (Team:SIEM)

[elasticmachine]

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Build Cause: Pull request #23966 updated

• Start Time: 2021-02-22T21:26:02.761+0000

• Duration: 32 min 16 sec

• Commit: 9bc1d4f

Test stats 🧪

Test	Results
Failed	0
Passed	1003
Skipped	217
Total	1220

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	1003
Skipped	217
Total	1220

[ChrsMark]

@urso this one affects master build, do you mind giving a +1 so as to proceed?

[urso]

There is already a skip on windows with a different github issue. The two issues seem to be duplicates.

Skimming the test code I noticed that a go-routine is started that is supposed to write the files. The test seems to be sensitive to timing + there is a chance of the test leaking a go-routine.

Does the test fail due to timing constraints and slow IO? What happens to the test if we add a time.Sleep(2 * time.Minute) in the helper go-routine (on sleep before and after each line in the go-routine)?

[ChrsMark]

@urso I wonder why we should add the sleep within the goroutine and not just right after it so as to give it time to do the IO ops and then let the main test to check for the events. I actually applied this on the latest commit(b10fe6c) and seems to work.

[urso]

we don't know how long we have to wait for the IO. The time.Sleep was for you to test if the test is too time sensitive... as evidence, not to fix the test.

e.g.

	go func() {
        time.Sleep(2 * time.Minute)
		ioutil.WriteFile(createdFilepath, []byte("hello world"), 0600)
        time.Sleep(2 * time.Minute)
		ioutil.WriteFile(updatedFilepath, []byte("hello world"), 0600)
        time.Sleep(2 * time.Minute)
	}()

If the test fails with these timeouts, it is a good indicator that the test is too sensitive to IO scheduler timings (which can easily block write operations for a minute if the system is overloaed).

If adding a time.Sleep after the go-routine really stabilizes the test the question is: why do we need to have the go-routine in the first place?

@andrewkroh Any idea if the go-routine is required for the test?

[andrewkroh]

I don't see why there is a goroutine with only WriteFiles. I expected there to be a time.Sleep in the goroutine followed by the WriteFile similar to https://github.com/elastic/beats/blame/b10fe6cf95f19881705c8ac0899e4f1aad9db3bf/auditbeat/module/file_integrity/metricset_test.go#L49-L53. I think it was a basic way to schedule some activity to happen after the MetricSet is running.

[urso]

Thank you. In that we should do the WriteFile without the go-routine.

[urso]

Better replace the go-routine with a code block:

		ioutil.WriteFile(createdFilepath, []byte("hello world"), 0600)
		ioutil.WriteFile(updatedFilepath, []byte("hello world"), 0600)

WriteFile returns when the file is written... no need to block the test suite for an extra 2 minutes.

[urso]

The test is still skipped on windows. Shall we unskip and see how it goes?

[urso]

@ChrsMark What's the status of this change? This is still one of the most flaky tests we need to address.

[ChrsMark]

@ChrsMark What's the status of this change? This is still one of the most flaky tests we need to address.

Seems there were some lint issues, I updated again, let's see if CI goes green now.