Skip to content
This repository was archived by the owner on Sep 21, 2023. It is now read-only.

Fix integration tests#259

Merged
leehinman merged 1 commit intoelastic:mainfrom
leehinman:244_integration_test
Mar 2, 2023
Merged

Fix integration tests#259
leehinman merged 1 commit intoelastic:mainfrom
leehinman:244_integration_test

Conversation

@leehinman
Copy link
Copy Markdown
Contributor

@leehinman leehinman commented Feb 24, 2023

What does this PR do?

updates integration tests to work with changes brought in with v2 protocol

  • switched to configuration layout mandated by v2
  • added mage test target that will run both unit and integration tests
  • switched CI runs from just running unit tests to running all tests
  • Skipped integration tests that require docker to run logstash or kafka, support for doing that is finalized in magefile.
  • updated example config file

Why is it important?

we need integration tests :-)

Checklist

  • My code follows the style guidelines of this project
  • 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 added an entry in CHANGELOG.md or CHANGELOG-developer.md.

How to test this PR locally

mage test

Related issues

@leehinman leehinman requested a review from a team as a code owner February 24, 2023 21:07
@leehinman leehinman requested review from belimawr and cmacknz and removed request for a team February 24, 2023 21:07
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Feb 24, 2023

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @leehinman? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Feb 24, 2023 •
edited
Loading

💚 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-03-01T23:45:33.695+0000

  • Duration: 17 min 22 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

@leehinman leehinman requested review from faec and fearful-symmetry and removed request for belimawr February 24, 2023 23:22
@leehinman leehinman force-pushed the 244_integration_test branch 3 times, most recently from 83eff8b to 4f0336a Compare February 27, 2023 16:24
@leehinman leehinman force-pushed the 244_integration_test branch from 4f0336a to 6322f97 Compare February 27, 2023 16:45
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Feb 27, 2023

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 244_integration_test upstream/244_integration_test
git merge upstream/main
git push upstream 244_integration_test

@leehinman leehinman force-pushed the 244_integration_test branch 6 times, most recently from 1257772 to 6b54843 Compare February 27, 2023 22:10
fearful-symmetry
fearful-symmetry reviewed Feb 27, 2023
View reviewed changes
controller/run_test.go Outdated
if err != nil && errors.Is(err, fs.ErrNotExist) {
continue
}
con, err := net.Dial("unix", serverAddr)
Copy link
Copy Markdown
Contributor

@fearful-symmetry fearful-symmetry Feb 27, 2023

Choose a reason for hiding this comment

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

Now that I think of it, if we're going to run the dial call in a loop, is the os.Stat needed? if there's an issue with the file, it will get caught by the Dial call.

Copy link
Copy Markdown
Contributor Author

@leehinman leehinman Feb 28, 2023

Choose a reason for hiding this comment

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

yeah, this can be simplified. I'll do a last pass once it all works on Unix, Mac & Windows.

@leehinman leehinman force-pushed the 244_integration_test branch 2 times, most recently from a0a5b17 to 11ffbb4 Compare February 28, 2023 17:06
@fearful-symmetry
Copy link
Copy Markdown
Contributor

fearful-symmetry commented Feb 28, 2023

Alright, tested locally:

DONE 34 tests, 4 skipped in 10.808s
>> go test: Integration Tests : Test Passed

It looks like some of those skipped tests are just marked as SkipNow(). Are they still in development?

@leehinman leehinman force-pushed the 244_integration_test branch 4 times, most recently from ab0f8e9 to 267ea12 Compare February 28, 2023 22:29
@leehinman leehinman force-pushed the 244_integration_test branch 3 times, most recently from 46f3fc8 to a4e5a98 Compare March 1, 2023 03:06
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 1, 2023

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 244_integration_test upstream/244_integration_test
git merge upstream/main
git push upstream 244_integration_test

@leehinman leehinman force-pushed the 244_integration_test branch 3 times, most recently from b3b9652 to 3091894 Compare March 1, 2023 22:22
@leehinman
Fix integration tests
ac7f1e1 
- switched to configuration layout mandated by v2
- added `mage test` target that will run both unit and integration
  tests
- switched CI runs from just running unit tests to running all tests
- Skipped integration tests that require docker to run logstash or
  kafka, support for doing that is finalized in magefile.
- updated example config file
- add support for running under Windows with named pipes

Closes elastic#244
@leehinman leehinman force-pushed the 244_integration_test branch from 3091894 to ac7f1e1 Compare March 1, 2023 23:45
@leehinman leehinman merged commit aefda0f into elastic:main Mar 2, 2023
@leehinman leehinman deleted the 244_integration_test branch March 2, 2023 03:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@cmacknz cmacknz Awaiting requested review from cmacknz cmacknz is a code owner automatically assigned from elastic/elastic-agent-data-plane

2 more reviewers

@fearful-symmetry fearful-symmetry fearful-symmetry left review comments

@faec faec faec approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@leehinman leehinman

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fix mage test:integration

4 participants

@leehinman @elasticmachine @fearful-symmetry @faec