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.

Sign up for GitHub

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

Filebeat pytest improvements #30103

Merged
merged 6 commits into from
Feb 1, 2022
Merged

Filebeat pytest improvements #30103

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
033aec0
Include the test_file as wait_until name
matschaffer Jan 31, 2022
35fb543
Support TESTING_FILEBEAT_FILEPATTERN to test a single ingest file
matschaffer Jan 31, 2022
6065ac4
Include full event on "not documented" case
matschaffer Jan 31, 2022
cd55578
Add changelog entry
matschaffer Jan 31, 2022
6ba47b2
Merge remote-tracking branch 'upstream/master' into filebeat-pytest-i…
matschaffer Jan 31, 2022
94130de
Lint fix
matschaffer Jan 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG-developer.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ The list below covers the major changes between 7.0.0-rc2 and master only.
- Whitelist `GCP_*` environment variables in dev tools {pull}28364[28364]
- Add support for `credentials_json` in `gcp` module, all metricsets {pull}29584[29584]
- Add gcp firestore metricset. {pull}29918[29918]
- Added TESTING_FILEBEAT_FILEPATTERN option for filebeat module pytests {pull}30103[30103]

==== Deprecated

Expand Down
4 changes: 2 additions & 2 deletions filebeat/tests/system/test_modules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def load_fileset_test_cases():
continue

test_files = glob.glob(os.path.join(modules_dir, module,
fileset, "test", "*.log"))
fileset, "test", os.getenv("TESTING_FILEBEAT_FILEPATTERN", "*.log")))
Copy link
Contributor

Choose a reason for hiding this comment

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

@ph @cmacknz General problem we have with these environment variables is that everyone that worked on the test suite knows about these, everyone else has to find them in the code base ... We should find a better place (not related to this PR).

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree

I think we should rethink how and why we introduce variables in the first place. The problem today is there are multiple ways to run tests that support or require different side effects.

I strongly think the test cli should give me all the required information for options without looking in the documentation.

Copy link
Contributor Author

@matschaffer matschaffer Feb 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Definitely agreed.

Not sure if pytest offers something like this directly. mage pythonIntegTest has inline help, but does a lot of extra work (15min for my laptop).

I guess maybe one option would be a lighter mage target with these options covered in the inline help.

In working on the tests I also noticed some similarity with metricbeat testdata integration tests. For example:

beats/metricbeat/module/couchbase/bucket/bucket_test.go

Line 36 in e6839ab

func TestFetchEventContents(t *testing.T) {

If we did these filebeat module tests as regular golang test cases, stuff like TESTING_FILEBEAT_FILEPATTERN would just be go test -run (pattern)

Copy link
Contributor

Choose a reason for hiding this comment

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

Having quicker mage task is an option.

I would like to remove all python integration tests and use a single language.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel that. 🧡

for test_file in test_files:
test_cases.append([module, fileset, test_file])

Expand Down Expand Up @@ -216,7 +216,7 @@ def run_on_file(self, module, fileset, test_file, cfgfile):
error_line)

# Make sure index exists
self.wait_until(lambda: self.es.indices.exists(self.index_name))
self.wait_until(lambda: self.es.indices.exists(self.index_name), name="indices present for {}".format(test_file))

self.es.indices.refresh(index=self.index_name)
# Loads the first 100 events to be checked
Expand Down
2 changes: 1 addition & 1 deletion libbeat/tests/system/beat/beat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ def is_documented(key, docs):
# Range keys as used in 'date_range' etc will not have docs of course
isRangeKey = key.split('.')[-1] in ['gte', 'gt', 'lte', 'lt']
if not(is_documented(key, expected_fields) or metaKey or isRangeKey):
raise Exception("Key '{}' found in event is not documented!".format(key))
raise Exception("Key '{}' found in event ({}) is not documented!".format(key, str(evt)))
if is_documented(key, aliases):
raise Exception("Key '{}' found in event is documented as an alias!".format(key))

Expand Down