You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Initially we only had two types of test runners: pipeline and system. Both operated at the data stream level (as opposed to the package level). Then we introduced a third type of test runner: asset. This one operated at the package level. To make this work with the existing test runner implementation, we had to introduce a TestRunner interface with a method named CanRunPerDataStream, which feels a bit smelly.
More recently, we've introduced a feature of letting tests be skippable via a configuration block in a test definition. While implementing this feature, @mtojek observed how it could be quite easy in the current test runner implementation to forget to add this feature for future test runner types. More discussion here: https://github.com/elastic/elastic-package/pull/221/files#r565955299.
It's starting to become apparent that the original test runner implementation might be ripe for a refactor. This issue is to track this change.
The text was updated successfully, but these errors were encountered:
The situation is getting worse, we have now also benchmark runners, that follow the same approach and have some code duplicity with the system test runners. When working on #1549 I found that I had to add the same code in multiple copies of the same functionality. We should probably prioritize some refactors around this area focused on reusing code.
ccing @kpollich who was tracking technical debt in the team.
Initially we only had two types of test runners:
pipeline
andsystem
. Both operated at the data stream level (as opposed to the package level). Then we introduced a third type of test runner:asset
. This one operated at the package level. To make this work with the existing test runner implementation, we had to introduce aTestRunner
interface with a method namedCanRunPerDataStream
, which feels a bit smelly.More recently, we've introduced a feature of letting tests be skippable via a configuration block in a test definition. While implementing this feature, @mtojek observed how it could be quite easy in the current test runner implementation to forget to add this feature for future test runner types. More discussion here: https://github.com/elastic/elastic-package/pull/221/files#r565955299.
It's starting to become apparent that the original test runner implementation might be ripe for a refactor. This issue is to track this change.
The text was updated successfully, but these errors were encountered: