Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid duplication configuration within functional tests #11865

Closed
chrisronline opened this issue May 17, 2017 · 0 comments
Closed

Avoid duplication configuration within functional tests #11865

chrisronline opened this issue May 17, 2017 · 0 comments

Comments

@chrisronline
Copy link
Contributor

Based on the comments in this PR, we should try and avoid potential duplicate configuration patterns where changes in one area might break tests in another area, in an non-obvious manner.

For example, if we're updating uiSettings with a default index pattern, assuming that default index pattern will always be logstash-* might cause unintended test failures if that ever changes. Ideally, these are all sourced from the same default configuration location.

On a slightly different note, it might be worthwhile to also consider the same shared configuration for known routes, since these are currently hard-coded in multiple functional tests.

Comments from PR:

chrisronline 36 minutes ago Member
Maybe not a better way, but does this defaultIndex need to match anything else that is setup in the test runner? I'd just want to avoid a scenario where this value is configured somewhere, it's copied here, but then someone possibly changes the value later in the future and this test breaks and it's not clear why. It's entirely possible that I'm missing something but just wanted to point it out in case it could happen

ycombinator 27 minutes ago Member
Yeah, I see what you mean. There's an implicit dependency here between this line and the await esArchiver.load('discover'); line a couple of lines further down. It would probably be better to create a wrapper method somewhere like loadDefaultIndex that loads the actual data using esArchiver.load but also sets up the defaultIndex value, and then use that wrapper method here.
All that said, this here is a pattern that is already established and being used in several places. So I suggest we open an issue for fixing this across the board rather than increasing the scope of this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants