Skip to content

Conversation

dmartinol
Copy link
Collaborator

First bunch of tests to address #1749
It also depends on #2016 which should be committed first

  • Fixes never-ending reconciliation loop by comparing the status fields instead of always updating the updated status (this would cause an immediate reconciliation)
  • Adds a test/e2e/operator/registry_lifecycle_test.go initial test to prove the capabilities of ginkgo as the testing framework for orchestrated workflows

Questions to be answered:

  • Where do we keep this files, is the current location suitable?
  • Should we call them from the operator-e2e-test task?

@dmartinol dmartinol requested a review from jhrozek September 26, 2025 15:28
Copy link

codecov bot commented Sep 26, 2025

Codecov Report

❌ Patch coverage is 31.25000% with 143 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.52%. Comparing base (718e2cb) to head (f93e3b3).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...thv-operator/controllers/mcpregistry_controller.go 0.00% 80 Missing ⚠️
cmd/thv-operator/pkg/sync/manager.go 61.85% 25 Missing and 12 partials ⚠️
...md/thv-operator/pkg/mcpregistrystatus/collector.go 10.34% 25 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2017      +/-   ##
==========================================
- Coverage   48.64%   48.52%   -0.12%     
==========================================
  Files         240      240              
  Lines       30264    30344      +80     
==========================================
+ Hits        14721    14725       +4     
- Misses      14439    14504      +65     
- Partials     1104     1115      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dmartinol dmartinol marked this pull request as ready for review October 2, 2025 17:06
@dmartinol
Copy link
Collaborator Author

I've added more tests covering most of the sync, data sources and filtering options and workflows.
I think it should be enough for now, especially if we're going to plan for some extensive review of the controller, like moving some of the functions to the thv-registryapi` service.

dmartinol and others added 12 commits October 3, 2025 10:39
- Create Ginkgo-based test suite for operator testing
- Add comprehensive test helpers for MCPRegistry operations
- Include test fixtures with sample YAML manifests
- Set up Kubernetes test environment with envtest support
- Add namespace isolation and cleanup utilities

🤖 Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Daniele Martinoli <[email protected]>
Co-authored-by: Claude <[email protected]>
- Create test suite with proper Kubernetes environment setup
- Add specialized helper utilities for MCPRegistry operations
- Implement ConfigMap test helpers for registry data validation
- Add status validation helpers for phase and condition checking
- Create timing utilities with proper timeout configurations
- Add test data factories for generating test resources
- Include builder patterns for fluent resource construction
- Support both ToolHive and upstream MCP registry formats
- Add comprehensive test fixtures and scenarios

🤖 Generated with [Claude Code](https://claude.ai/code)

Signed-off-by: Daniele Martinoli <[email protected]>
Co-authored-by: Claude <[email protected]>
- Fix finalizer removal using Patch instead of Update to avoid resource conflicts
- Update registry data structure to match expected schema (add required fields: tier, status, tools, image)
- Add proper registry deletion waiting in cleanup to prevent namespace deletion issues
- Fix lint errors by removing dot imports from non-test files
- Add comprehensive MCPRegistry lifecycle test coverage
- Improve error handling and logging in test helpers

Signed-off-by: Daniele Martinoli <[email protected]>
Co-authored-by: Claude <[email protected]>

🤖 Generated with [Claude Code](https://claude.ai/code)
… correct initial status"

Signed-off-by: Daniele Martinoli <[email protected]>
- Introduced environment variable handling for KUBEBUILDER_ASSETS
- Added warning for missing kubebuilder assets to improve test reliability
- Updated test environment configuration to include BinaryAssetsDirectory

This change aims to streamline the e2e testing process and provide clearer feedback on asset availability.

Signed-off-by: Daniele Martinoli <[email protected]>
- initial draft of e2e tests

Signed-off-by: Daniele Martinoli <[email protected]>
Copy link
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

I was mostly focusing on the controller changes which look good with one question about the has. In the test code, the question is just whether we want to remove some of the unused code or whether you want to keep it for later.

jhrozek
jhrozek previously approved these changes Oct 6, 2025
Copy link
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

I acked this patch if you want to merge now - I know on Slack you said you also want to integrate with Taskfile targets, but let me know if this should be done later.

Signed-off-by: Daniele Martinoli <[email protected]>
Signed-off-by: Daniele Martinoli <[email protected]>
Signed-off-by: Daniele Martinoli <[email protected]>
Signed-off-by: Daniele Martinoli <[email protected]>
…ciliation and avoid errors due to updated resource.

- reviewed overall status calculation to consider updates or (if nil) lates status

Signed-off-by: Daniele Martinoli <[email protected]>
@dmartinol
Copy link
Collaborator Author

all tests passed @jhrozek

Copy link
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

epic! Let's merge

@dmartinol dmartinol requested a review from jhrozek October 7, 2025 14:46
@dmartinol dmartinol enabled auto-merge (squash) October 7, 2025 14:46
@dmartinol dmartinol merged commit 130be28 into stacklok:main Oct 7, 2025
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants