Skip to content

Conversation

@djptek
Copy link
Contributor

@djptek djptek commented Sep 7, 2022

What does this PR do?

This PR will enhance system testing through the implementation of an assertion for expected number of events.

Checklist

  • unpack events embedded in a message/data/events/etc. field within a hit
  • add test case to httpjson integration package
  • update readme
  • ensure test does not fail when test parameters are absent
  • [ ] handle persistence between calls NOT YET REQUIRED
  • [ ] sum events across calls IMPLICIT

Author's Checklist

  • run system tests vs elasticsearch + 4 randomly selected integrations to check for regressions
    • 1password
    • [ ] cisco_aironet
    • elasticsearch
    • hadoop
    • [ ] juniper_junos
    • [ ] kafka_log
    • nginx
    • traefik
    • [ ] zookeeper
  • add test under other

How to test this PR locally

Pull this branch and install elastic-package with make install

Clone elastic/integrations

Configure the httpjson (or related) integration as follows

For an http request, consider this example from the httpjson/generic data stream's test-expected-hit-count-config.yml, shown below.

input: httpjson
service: httpjson
data_stream:
  vars:
    data_stream.dataset: httpjson.generic
    username: test
    password: test
    request_url: http://{{Hostname}}:{{Port}}/testexpectedhits/api
    response_split: |- 
      target: body.hits
      type: array
      keep_parent: false     
assert:
  hit_count: 4

The data_stream.vars.request_url corresponds to a test-stub path in the _dev/deploy/docker/files/config.yml file.

  - path: /testexpectedhits/api
    methods: ["GET"]
    request_headers:
      Authorization:
        - "Basic dGVzdDp0ZXN0"
    responses:
      - status_code: 200
        headers:
          Content-Type:
            - "application/json; charset=utf-8"
        body: |-
          {"total":3,"hits":[{"message": "success"},{"message": "success"},{"message": "success"}]}

As assert.hit_count is defined and > 0 the system test will only pass the assertion when the number of elements in the top-level array of the JSON body is equal to this value. In this case the assertion requires 4 hits when the actual count will be 3, so the assertion should fail.

Run the system tests using elastic-package test system -v

The test results should include the following line:

│ httpjson │ generic     │ system    │ expected-hit-count │ FAIL: observed hit count 3 did not match expected hit count 4 │ 21.123980175s │

Next, amend assert.hit_count so that this matches the number of elements (3) in the response to the /testexpectedevents/api path. The test should now pass as follows

│ httpjson │ generic     │ system    │ expected-hit-count │ PASS   │ 20.979543465s │

Related issues

## Screenshots

@djptek djptek added the enhancement New feature or request label Sep 7, 2022
@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 7, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-02-07T21:29:53.733+0000

  • Duration: 32 min 13 sec

Test stats 🧪

Test Results
Failed 0
Passed 885
Skipped 0
Total 885

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 7, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (35/35) 💚
Files 65.909% (87/132) 👍
Classes 61.376% (116/189) 👍
Methods 48.718% (399/819) 👎 -0.054
Lines 31.73% (3576/11270) 👎 -0.042
Conditionals 100.0% (0/0) 💚

@djptek djptek self-assigned this Sep 8, 2022
@djptek djptek marked this pull request as ready for review November 22, 2022 16:52
@djptek djptek requested a review from andrewkroh November 22, 2022 16:52
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Thanks for adding this. System tests are a bit unpredictable by nature, so counting events may be a bit flaky, though I guess we are fine in cases where we control the source events generated.

Could you add a test package that leverages this feature?

@djptek
Copy link
Contributor Author

djptek commented Nov 29, 2022

@jsoriano

Could you add a test package that leverages this feature?

I added to httpjson see: elastic/integrations#4152

@jsoriano
Copy link
Member

@jsoriano

Could you add a test package that leverages this feature?

I added to httpjson see: elastic/integrations#4152

Sorry, I meant a test package here in elastic-package, you can add it to test/packages, or modify one of the ones there.

@djptek djptek marked this pull request as draft December 2, 2022 17:33
@jsoriano
Copy link
Member

/test

@djptek
Copy link
Contributor Author

djptek commented Dec 23, 2022

@jsoriano

Thanks for adding this. System tests are a bit unpredictable by nature, so counting events may be a bit flaky, though I guess we are fine in cases where we control the source events generated.

Could you add a test package that leverages this feature?

I've added a (passing) test package test/packages/other/hit_count_assertion/

To provoke a fail, I changed

test/packages/other/hit_count_assertion/data_stream/test/_dev/test/system/test-hits-config.yml: hit_count: 100

to

test/packages/other/hit_count_assertion/data_stream/test/_dev/test/system/test-hits-config.yml: hit_count: 101

Which yields the following output in CI test make install test-check-packages-other check-git-clean

[2022-12-23T08:51:32.907Z] 2022/12/23 08:51:32 DEBUG found 100 hits in logs-hit_count_assertion.test-ep data stream
[2022-12-23T08:51:33.007Z] 2022/12/23 08:51:32 DEBUG assert hit count expected 101, observed 100
...
[2022-12-23T08:51:48.363Z] Error: one or more test cases failed

Restoring the value of 100 allows the test to pass.

@djptek djptek marked this pull request as ready for review December 23, 2022 10:02
@djptek djptek requested review from andrewkroh and jsoriano January 11, 2023 17:43
@ebeahan
Copy link
Member

ebeahan commented Feb 6, 2023

/test

@ebeahan
Copy link
Member

ebeahan commented Feb 8, 2023

@jsoriano thanks for taking another look and for the feedback. I think the CI issues are resolved.

@ebeahan
Copy link
Member

ebeahan commented Feb 9, 2023

@jsoriano I believe I need a final ✅ if everything looks ok from your side.

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Sorry for the delay!

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants