Skip to content

Make CI run agent tests too for now#2806

Merged
artem-shelkovnikov merged 6 commits intomainfrom
artem/add-agent-tests-to-ci
Sep 13, 2024
Merged

Make CI run agent tests too for now#2806
artem-shelkovnikov merged 6 commits intomainfrom
artem/add-agent-tests-to-ci

Conversation

@artem-shelkovnikov
Copy link
Member

@artem-shelkovnikov artem-shelkovnikov commented Sep 6, 2024

Closes https://github.com/elastic/search-team/issues/8207

With this change CI will run tests for the agent component along with regular tests. That'll help us find issues with protobuf bumps and general problems with components and prevent merging broken changes to main.

Checklists

Pre-Review Checklist

  • this PR does NOT contain credentials of any kind, such as API keys or username/passwords (double check config.yml.example)
  • this PR has a meaningful title
  • this PR links to all relevant github issues that it fixes or partially addresses
  • if there is no GH issue, please create it. Each PR should have a link to an issue
  • this PR has a thorough description
  • Covered the changes with automated tests
  • Tested the changes locally
  • Added a label for each target release version (example: v7.13.2, v7.14.0, v8.0.0)

@artem-shelkovnikov artem-shelkovnikov marked this pull request as ready for review September 9, 2024 08:17
@artem-shelkovnikov artem-shelkovnikov requested a review from a team September 9, 2024 08:17
Copy link
Member

@seanstory seanstory left a comment

Choose a reason for hiding this comment

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

I had approved, but realized separately that this module didn't have an __init__.py file. When I added it, it suddenly broke coverage for make test. We may need to find a way to separate the coverage settings between these two targets, and that should probably be introduced as part of this PR. See: #2811

@artem-shelkovnikov artem-shelkovnikov force-pushed the artem/add-agent-tests-to-ci branch from 8e3c507 to f9750d1 Compare September 10, 2024 08:32
@seanstory seanstory self-requested a review September 10, 2024 16:34
Comment on lines 27 to 29
install-agent: install
.venv/bin/pip install -r requirements/agent.txt

Copy link
Member

Choose a reason for hiding this comment

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

I vote we leave this, but just have it alias to .venv/bin/elastic-ingest for now. That way the agent code doesn't need to be changed again, and we have a place we can hook into in the future for agent-specific bits when we need/want them. Like:

install-agent: .venv/bin/elastic-ingest
    # nothing here

WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, just added it back in last commit

@artem-shelkovnikov artem-shelkovnikov enabled auto-merge (squash) September 13, 2024 09:10
@artem-shelkovnikov artem-shelkovnikov merged commit 278c92c into main Sep 13, 2024
@artem-shelkovnikov artem-shelkovnikov deleted the artem/add-agent-tests-to-ci branch September 13, 2024 09:17
github-actions bot pushed a commit that referenced this pull request Sep 13, 2024
@github-actions
Copy link

💚 Backport PR(s) successfully created

Status Branch Result
8.x #2823

This backport PR will be merged automatically after passing CI.

github-actions bot added a commit that referenced this pull request Sep 13, 2024
Co-authored-by: Artem Shelkovnikov <artem.shelkovnikov@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants