Defer cleanup flag

[mtojek]

This PR modifies the condition for sleeping before tear down and makes it test runner wide.

It adds a new command option: --defer-cleanup.

[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 #175 opened]

• Start Time: 2020-11-19T10:26:15.580+0000

• Duration: 9 min 7 sec

Test stats 🧪

Test	Results
Failed	0
Passed	9
Skipped	0
Total	9

[mtojek]

As expected it decreased the total execution time from ~9m to ~7m.

[ycombinator]

I like this very much — it decouples the cleanup sleep from the log level, making it useable regardless of log level. 👍

I noticed that there is a bit of code duplication between the pipeline test runner and the system test runner. When we introduce the Asset Test Runner in #168, we would have to duplicate the logic there as well. I wonder if we could somehow prevent this, maybe by making the teardown step part of the API of a test runner?

[mtojek]

I noticed that there is a bit of code duplication between the pipeline test runner and the system test runner. When we introduce the Asset Test Runner in #168, we would have to duplicate the logic there as well. I wonder if we could somehow prevent this, maybe by making the teardown step part of the API of a test runner?

Are you referring to the part of tearDown() which "sleeps"? So that, this part is common for every test runner?

[ycombinator]

Are you referring to the part of tearDown() which "sleeps"? So that, this part is common for every test runner?

Yes, I'm wondering if we can:

• make tearDown() => TearDown() so it's exported from every test runner and part of the Test Runner's API.
• then the caller of the test runner can sleep if needed (based on the --defer-cleanup flag) and then call TearDown().

Thinking about this more, this might be a lot easier to implement when the changes in #168 get merged, where I introduce an interface type for test runners to implement: https://github.com/elastic/elastic-package/pull/168/files#diff-348effe2edd1aca63ec29394369029f7844afde24cd36ce7210fae5c55729bb5R22.

So we can leave this PR as-is for now, or extract the interface type out of #168 into this PR, or do something else in this PR — up to you.

[mtojek]

Thinking about this more, this might be a lot easier to implement when the changes in #168 get merged, where I introduce an interface type for test runners to implement: https://github.com/elastic/elastic-package/pull/168/files#diff-348effe2edd1aca63ec29394369029f7844afde24cd36ce7210fae5c55729bb5R22.

Yes, I was thinking about the same. Could we postpone this refactor change then?

[ycombinator]

Yes, lets postpone it. I can implement it as part of #168 or extract the interface out of #168 into its own PR and implement it there. Sorry for the noise.

[ycombinator]

LGTM.