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

[Metricbeat] Mocked Tests: Do not halt if doc check fails #11780

Merged
merged 3 commits into from
Apr 15, 2019

Conversation

sayden
Copy link
Contributor

@sayden sayden commented Apr 12, 2019

With the current behaviour, tests halt if doc checks can't find a field and the -expected.json file is not written. With this change, the file is written at least, helping identifying the issue.

@ruflin I was wondering if descriptive errors which leads to the solution to developers and contributors are a way to go as a general rule. In this particular context, I was about to write a very long and descriptive error here because I realized that a field that exists in {module}/{metricset}/_meta/fields.yml can still be missing in root fields.yml and throw an error... which confused me for a while until I thought to try with a make update. WDYT?

@sayden sayden added enhancement Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Apr 12, 2019
@sayden sayden requested a review from ruflin April 12, 2019 11:39
@sayden sayden requested a review from a team as a code owner April 12, 2019 11:39
@sayden sayden self-assigned this Apr 12, 2019
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

+1 on making errors more descriptive, this is especially useful for community contributors and new joiners.

Could you rebase on master? I think I saw a PR that skipped the KB issue.

@sayden sayden force-pushed the feature/mb/do-not-halt-in-doc-check branch from 44f24c2 to b1995be Compare April 15, 2019 08:50
@sayden sayden merged commit 5836641 into elastic:master Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Metricbeat Metricbeat Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants