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.

Sign up for GitHub

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

Jump to bottom

test: reduce pod and communication dependencies #43

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

test: reduce pod and communication dependencies #43

wants to merge 10 commits into from

Conversation

maffettone
Copy link
Contributor

@maffettone maffettone commented May 13, 2024 •
edited
Loading

Description

Closes #42

  • Added offline utilities for doing smoke tests, that should also help with developing agents offline.
  • Refactored old utils for the sake of import consistency.
  • Rebuilt any test that depended on communication and scrubbed most all pytest xfail on timeouts.
  • Updated requirements. Specifically databroker V2 was always a requirement for agents.base.BaseAgent, but not in the requirements.txt.
  • Moved the BlueskyConsumer in adjudicators from an inheritance to an attribute. This will allow flexibility with other message busses, and makes testing less Kafka dependent.

Motivation and Context

Test suite was taking way too long, and fairly brittle. Tests should in general take less than 1 second to run.

Summary of Changes for Release Notes

Fixed

Added

Offline utilities for Kafka, QueueServer, Agents, Adjudicators

Changed

Moved databroker V2 to main requirements.

Removed

Test suite dependence on containers outside the tests that test communication specifically.

How Has This Been Tested?

Pytest.

@maffettone maffettone force-pushed the test-reduce-pod-dependencies branch 2 times, most recently from ec9bf60 to 0263749 Compare May 13, 2024 05:43
@maffettone maffettone force-pushed the test-reduce-pod-dependencies branch from 0263749 to 9590410 Compare May 13, 2024 05:50
@maffettone
Copy link
Contributor Author

Tests now take less time than installing dependencies. :D

@padraic-shafer
Copy link
Contributor

Is there a case for installing "dev" without "agents"? If not, consider adding ".[agents]" into "requirements-dev.txt".

I installed ".[dev]" and pytest failed because of missing torch, scikit-learn dependencies.

@maffettone
Copy link
Contributor Author

My thought there was mostly based on my development cycle, and the expense of putting the .[agents] into an env.
If I'm building a new feature (e.g., an agent in jax or new base features), I only run the tests that I expect to be effected, or the ones I have written to test the new feature.

But the CI/CD has to test everything. Let me know if this is not best practice.

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

@tacaswell tacaswell Awaiting requested review from tacaswell

@padraic-shafer padraic-shafer Awaiting requested review from padraic-shafer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Testing suite has too many dependencies on containerized services
2 participants
@maffettone @padraic-shafer