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

Replaces x-pack/test/functional/es_archives/logstash_functional with test/functional/fixtures/es_archiver/logstash_functional in test files #114189

Closed
wants to merge 3 commits into from

Conversation

bhavyarm
Copy link
Contributor

@bhavyarm bhavyarm commented Oct 6, 2021

We have two logstash functional data folders - one under test/functional/fixtures/es_archiver/logstash_functiona and another under x-pack/test/functional/es_archives/logstash_functional ( this data has mapping conflicts). This PR is replacing the latter with the former.

…irectory with the one from test/functional/fixtures data
@bhavyarm bhavyarm self-assigned this Oct 6, 2021
@bhavyarm bhavyarm added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes test_xpack_functional labels Oct 6, 2021
@LeeDr
Copy link

LeeDr commented Oct 7, 2021

It seems that both of those logstash_functional data sets have gotten small modifications to them. Here's the diff;

leedr@Lees-MacBook-Pro kibana % diff test/functional/fixtures/es_archiver/logstash_functional/data.json x-pack/test/functional/es_archives/logstash_functional/data.json
46,50d45
<       "nestedField": [
<         {
<           "child": "nestedValue"
<         }
<       ],
1432304c1432299
<       "agent": "Missing/Fields",
---
>       "agent": "Mozilla/5.0 (X11; Linux i686) AppleWebKit/534.24 (KHTML, like Gecko) Chrome/11.0.696.50 Safari/534.24",
1432337c1432332
<       "referer": null,
---
>       "referer": "http://www.slate.com/warning/kevin-p-chilton",
1432384a1432380,1432414
> 
> 
> {
>   "type": "doc",
>   "value": {
>     "id": "AU_x4AYpGFA8no6Qjlv4",
>     "index": "logstash-2015.09.22",
>     "source": {
>       "@message": "93.121.116.124 - - [2015-09-22T05:33:14.920Z] \"GET /styles/ads.css HTTP/1.1\" 200 5846 \"-\" \"Mozilla/5.0 (X11; Linux i686) AppleWebKit/534.24 (KHTML, like Gecko) Chrome/11.0.696.50 Safari/534.24\"",
>       "@tags": [
>         "success",
>         "info"
>       ],
>       "@timestamp": "2015-09-22T05:33:14.930Z",
>       "agent": "Mozilla/5.0 (X11; Linux i686) AppleWebKit/534.24 (KHTML, like Gecko) Chrome/11.0.696.50 Safari/534.24",
>       "bytes": 5846,
>       "clientip": "93.121.116.124",
>       "extension": "css",
>       "headings": [
>         "<h3>daniel-tani</h5>",
>         "http://www.slate.com/success/mark-kelly"
>       ],
>       "host": "cdn.theacademyofperformingartsandscience.org",
>       "index": "logstash-2015.09.22",
>       "referer": "https://www.taylorswift.com/",
>       "request": "/styles/ads.css",
>       "response": "200",
>       "spaces": "loook what    you made    me do    ",
>       "type": "apache",
>       "url": "https://cdn.theacademyofperformingartsandscience.org/styles/ads.css",
>       "utc_time": "2015-09-22T05:33:14.930Z",
>       "xss": "<script>console.log(\"xss\")</script>"
>     }
>   }
> }
\ No newline at end of file

@kibanamachine
Copy link
Contributor

kibanamachine commented Oct 7, 2021

⏳ Build in-progress, with failures

Failed CI Steps

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @bhavyarm

@bhavyarm
Copy link
Contributor Author

bhavyarm commented Oct 7, 2021

@flash1293
cc @LeeDr @mattkime

Question to you because you reviewed Wylie's PR - #76971

Did this pr because we realized there were two logstash functional data files. One under x-pack - 'x-pack/test/functional/es_archives/logstash_functional' and one under OSS - 'test/functional/fixtures/es_archiver/logstash_functional' As you can see except two test files - the changes are fine and tests pass for the rest of them.

Now - the logstash functional data in xpack has mapping conflict for this field - machine.ram_range.

Screen Shot 2021-10-07 at 5 34 07 PM

And you can see @LeeDr comment above which talks about other differences.

My question is do you know why Wylie added field mapping conflict? Was it done intentionally? Do we know how this affects tests? Thank you!

This is the mapping for the field in logstash-*

"ram_range" : {
              "properties" : {
                "gte" : {
                  "type" : "long"
                },
                "lt" : {
                  "type" : "long"
                }
              }
            }
          }
        },
 "ram_range" : {
              "properties" : {
                "gte" : {
                  "type" : "long"
                },
                "lt" : {
                  "type" : "long"
                }
              }
            }
          }
        },
 "ram_range" : {
              "type" : "long_range"
            }

@flash1293
Copy link
Contributor

@bhavyarm I think as long as just the xpack version is loaded, there’s no conflict. This test is there to check whether range type fields are shown in the correct places - this is a valid test we should keep.

@bhavyarm
Copy link
Contributor Author

@flash1293 Joe - loading this is showing conflicts - 'x-pack/test/functional/es_archives/logstash_functional'. For the field machine.ram_range as I have indicated in the screenshot.

@flash1293
Copy link
Contributor

Huh, I don't think this happens on purpose - the field got added to test the range field type integration, I think the conflict isn't necessary to do so.

@bhavyarm
Copy link
Contributor Author

@flash1293 Right. I am going to wait for Lee's come back and figure out if we want to upload data without conflicts here. Or add a test to test mapping conflict now that there is data. Thanks!

@bhavyarm
Copy link
Contributor Author

Closing as we have tests for data views and conflicts

@bhavyarm bhavyarm closed this Jun 28, 2022
@bhavyarm bhavyarm deleted the replaceDupLogstashFunctional branch June 28, 2022 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes test_xpack_functional v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants