Skip to content

[Metricbeat][ Add option to remove fields from comparison in mocked tests and mage target#11703

Closed
sayden wants to merge 0 commit intoelastic:masterfrom
sayden:feature/mb/add_mage_to_mocked-tests
Closed

[Metricbeat][ Add option to remove fields from comparison in mocked tests and mage target#11703
sayden wants to merge 0 commit intoelastic:masterfrom
sayden:feature/mb/add_mage_to_mocked-tests

Conversation

@sayden
Copy link
Copy Markdown
Contributor

@sayden sayden commented Apr 8, 2019

Added 3 things:

  • Config option remove_fields_from_comparison to remove fields from the JSON generated and expected files to byte-to-byte comparison when asserting tests.
  • Added a README.md file to the folder where the code is located showing current options, flags and use.
  • Added a mage target called mockedTests to avoid navigating to beats/metricbeat/mb/testing/data to run tests.

The PR can be considered a WIP but some overlook might be nice @ruflin to know if you like the current heading and altitude 😉

@sayden sayden added enhancement Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Apr 8, 2019
@sayden sayden self-assigned this Apr 8, 2019
@sayden sayden requested a review from ruflin April 8, 2019 18:00
@sayden sayden requested a review from a team as a code owner April 8, 2019 18:00
Comment thread metricbeat/magefile.go Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There's a Package field in GoTestArgs type but the order of the executed contents isn't correct when using it. By placing everything as ExtraFlag I could control the order of the flags and package run.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably @andrewkroh knows best here.

Comment thread metricbeat/mb/testing/data/data_test.go Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ruflin I modified this from MarshalIndent to just Marshal. I understood that the idea of using indent was to write into the data.json file in pretty format but for comparison it wasn't nice when it failed as the console was escaping newlines like here
image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No strong preference as long as the files written to disk are indented.

For the screenshot you posted: It shows the error pretty well I would say.

Comment thread metricbeat/mb/testing/data/example.yml Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added some example config file for full reference :)

@sayden sayden added the in progress Pull request is currently in progress. label Apr 9, 2019
Copy link
Copy Markdown
Contributor

@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.

Thank your for the addition of the docs.

I suggest to take out docs and magefile change into a separate PR for further discussion so we can get the field remove code in quickly to rebase the apache module on top of it. WDYT?

Comment thread metricbeat/mb/testing/data/data_test.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No strong preference as long as the files written to disk are indented.

For the screenshot you posted: It shows the error pretty well I would say.

Comment thread metricbeat/magefile.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably @andrewkroh knows best here.

Comment thread metricbeat/mb/testing/data/README.md Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's not correct that the server is launched on 5555, it's a random port selected but we hardcode 5555 in the final event.

@sayden
Copy link
Copy Markdown
Contributor Author

sayden commented Apr 9, 2019

Closed this by mistake, I cannot reopen it now 🤓

PR has been split in 3:

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

Labels

enhancement in progress Pull request is currently in progress. Metricbeat Metricbeat Team:Integrations Label for the Integrations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants