Skip to content
This repository was archived by the owner on Oct 28, 2024. It is now read-only.

Conversation

@v1v
Copy link
Member

@v1v v1v commented Jul 16, 2020

What does this PR do?

Add some test cases to validate a multimodule beat with some exclusions.

Why is it important?

Avoid the corner case of detecting a quite generic regex that should not apply for another beats

Related issues

Caused by elastic/beats#19985 and therefore elastic/beats#19986

@v1v v1v self-assigned this Jul 16, 2020
@v1v v1v added the automation label Jul 16, 2020
@ghost
Copy link

ghost commented Jul 16, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #671 updated]

  • Start Time: 2020-07-16T11:06:49.424+0000

  • Duration: 9 min 13 sec

Test stats 🧪

Test Results
Failed 0
Passed 687
Skipped 9
Total 696

@v1v v1v marked this pull request as ready for review July 16, 2020 10:26
@botelastic botelastic bot added the groovy label Jul 16, 2020
@v1v v1v requested review from a team and andrewkroh July 16, 2020 10:27
@v1v v1v added the test label Jul 16, 2020
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd try to use a name explaining the corner case, adding the PR id in a Java comment, then the method would be self-explanatory

Copy link
Member Author

Choose a reason for hiding this comment

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

I've decided to use a more meaningful test case name, since the cornercase might be misleading

@v1v v1v changed the title [test] add tests for cornercases [test] add more tests for the gitMatchingGroup step Jul 16, 2020
Copy link
Contributor

Choose a reason for hiding this comment

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

i.e. : test_multiple_groups_with_multiple_modules_does_not_capture_module

It's more verbose but clearer IMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this use case is almost the same than the above ones, as the exclude pattern includes a subdirectory. I would name it then: test_module_detection_with_path_in_pattern

@v1v v1v merged commit cfa1197 into elastic:master Jul 17, 2020
@v1v v1v deleted the feature/uts-to-support-module-for-cornercases branch July 17, 2020 06:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants