-
Notifications
You must be signed in to change notification settings - Fork 42
work for issue #379 - adding back rpm and deb tests #439
Conversation
@mdelapenya when I run Centos, it works super. But when I run Debian, it throws an error and I can't trace the logging or the go code itself to be sure. I could use help, please. The error cited in the logs looks to relate exactly to the error we were attempting to fix in the cited issue above. perhaps it is just not fixed yet? But also, perhaps, our code needs some modification. I'd like to clarify what it is intending tho... we use a Debian image, with systemd - but the scripting indicates it is using some sysv init usage so, I'm not sure what is going on. This one line is particularly confusing from the logging below:
|
💔 Tests Failed
Expand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
|
Test | Results |
---|---|
Failed | 1 |
Passed | 42 |
Skipped | 20 |
Total | 63 |
Genuine test errors
💔 There are test failures but not known flaky tests, most likely a genuine test failure.
- Name:
Initializing / End-To-End Tests / helm_apm-server / The APM Server chart will create recommended K8S resources – APM Server
@@ -15,6 +15,19 @@ Examples: | |||
| centos | tar | | |||
| debian | tar | | |||
|
|||
@enroll |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI - I wanted to keep this separate from the above 'install' step due to the underlying sub-command tested. it would be a different failure if this broke but the other did not.
/package |
I don't know how to re-trigger tests apart from going into Jenkins and logging in and doing a re-build. that isn't too bad tho, but it is interesting that subsequent PR pushes don't auto retrigger - might be good to put that in, else it keeps us on the honor system which unintentionally may fail us. I pushed a new change and it didn't re-trigger which is what made me think of it. Perhaps we could support something like /git-bot test this please? Also - I have no idea why a pre-submit test is failing, help? |
About the pre-submit errors, they are related to gherkin lint. Could you fix this line?
From: When the "elastic-agent" process is "restarted" on the host
And the "filebeat" process is in the "started" state on the host To: When the "elastic-agent" process is "restarted" on the host
Then the "filebeat" process is in the "started" state on the host
|
To run the tests with a GH comment, you can check: https://github.com/elastic/e2e-testing/blob/master/.ci/Jenkinsfile#L27 |
@mdelapenya its ready for review, but I had to feel-out how the linter would react. Can you help point me to the linting guidelines I can read thru? |
should be all good to go now, if not, we can review what failed (test or lint!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @EricDavisX this LGTM! Great job with the Gherkin sequencing! Let's see how it goes with test failures and flakiness before merging this.
Let's also discuss (maybe in another issue) if we move the systemd-specific
scenario to a separate feature file, not added to the CI parallel branches.
Hi, I've pushed up latest changes which:
here is evidence of my validating the skip works: Manu this also closes the 'skip' support request as logged in: |
This reverts commit a30e1f4.
* adding back rpm and deb tests * arbitrary change to trigger ci tests * fix linting * fixing more linting * fully exploring gherkin linting apparently * final linting fix * updating ci suites to match new tags * reset indention w feedback passes linting locally * adding skip support and skipping rpm and deb tests * back to fixing linting for tags now Co-authored-by: Manuel de la Peña <mdelapenya@gmail.com>
* adding back rpm and deb tests * arbitrary change to trigger ci tests * fix linting * fixing more linting * fully exploring gherkin linting apparently * final linting fix * updating ci suites to match new tags * reset indention w feedback passes linting locally * adding skip support and skipping rpm and deb tests * back to fixing linting for tags now Co-authored-by: Manuel de la Peña <mdelapenya@gmail.com>
* work for issue #379 - adding back rpm and deb tests (#439) * adding back rpm and deb tests * arbitrary change to trigger ci tests * fix linting * fixing more linting * fully exploring gherkin linting apparently * final linting fix * updating ci suites to match new tags * reset indention w feedback passes linting locally * adding skip support and skipping rpm and deb tests * back to fixing linting for tags now Co-authored-by: Manuel de la Peña <mdelapenya@gmail.com> * fix: use proper gherkin comments (#519) Co-authored-by: Eric Davis <eric.davis@elastic.co>
* work for issue #379 - adding back rpm and deb tests (#439) * adding back rpm and deb tests * arbitrary change to trigger ci tests * fix linting * fixing more linting * fully exploring gherkin linting apparently * final linting fix * updating ci suites to match new tags * reset indention w feedback passes linting locally * adding skip support and skipping rpm and deb tests * back to fixing linting for tags now Co-authored-by: Manuel de la Peña <mdelapenya@gmail.com> * fix: use proper gherkin comments (#519) Co-authored-by: Eric Davis <eric.davis@elastic.co>
What does this PR do?
2 things:
very small change to rename the @ tag to be more accurate per the tests being done and to sync 2 file names to the @ tag name for the whole file (I think this is our best practice pattern?)
most primarily, this is work for [Fleet] Add back test scenarios for .deb and .rpm systemd deploy syntax #379 - adding in the deb and rpm tests against centos & debian images that were removed prior.
I used existing .go implementation code that remained and which I think is functional and updated so far, only feature files.
Why is it important?
we need coverage over rpm and deb installs, even tho they are no longer the recommended usage - they are still supported and expected to be used.
In 379 you can see the discussion, we removed this accidentally / temporarily as we implemented new support for 'install' feature.
Checklist
- [ ] I have commented my code, particularly in hard-to-understand areas- [ ] I have made corresponding changes to the documentation- [ ] I have made corresponding change to the default configuration files- [ ] I have added tests that prove my fix is effective or that my feature works- [ ] I have run the Unit tests for the CLI, and they are passing locally- [ ] I have noticed new Go dependencies (runmake notice
in the proper directory)Author's Checklist
I actually am opening this as a draft to get some help from @mdelapenya or perhaps @blakerouse if we need
How to test this PR locally
OP_LOG_LEVEL=DEBUG DEVELOPER_MODE=true godog -t enroll > test-debug.log
Related issues
Follow-ups
So, Centos is working as submitted here, But Debian is throwing an error.. and I am not 100% sure I'm tracing the go code we've implemented right to know what question to ask. I'll put logs and more details in the comments and we can figure it out I hope.