Skip to content

Conversation

@manusa
Copy link
Member

@manusa manusa commented Dec 4, 2025

No description provided.

@manusa manusa added this to the 0.1.0 milestone Dec 4, 2025
defer watcher.mu.Unlock()
return len(watcher.lastKnownState.apiGroups) > 0
}), "timeout waiting for watcher to capture initial state")
}, watcherStateTimeout, eventuallyTick, "timeout waiting for watcher to capture initial state")
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the timeout value based on? its had multiple different values

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I messed with those while trying to fix the windows flakyness, hence the multiple changes.

At the moment it's 100ms which is double than the poll interval set in the test (50ms)

https://github.com/marcnuri-forks/kubernetes-mcp-server/blob/959a45d4a72e3bcf5ed521e927f6a08a21a82e41/pkg/kubernetes/watcher/cluster_test.go#L206-L208

Copy link
Collaborator

Choose a reason for hiding this comment

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

so you want to keep them at different values for each test?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I misunderstood your question. I see what you mean now, let me change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed, let me know if the new values make sense.

@manusa manusa marked this pull request as draft December 4, 2025 17:24
@manusa manusa marked this pull request as ready for review December 5, 2025 05:00
@manusa manusa requested a review from nader-ziada December 5, 2025 05:00
@nader-ziada
Copy link
Collaborator

lgtm

@nader-ziada nader-ziada merged commit 5f52c9d into containers:main Dec 5, 2025
6 checks passed
@manusa manusa deleted the test/refactor branch December 5, 2025 13:16
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