tests: refine coverage and add cache stats, settings enhancements, and provider fixes#137
tests: refine coverage and add cache stats, settings enhancements, and provider fixes#137RicherTunes wants to merge 6 commits intomainfrom
Conversation
…/remove/clear; add BrainarrSettings property tests
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| var s = new BrainarrSettings { Provider = AIProvider.Ollama }; | ||
| s.ConfigurationUrl = "localhost:11434"; | ||
| s.ConfigurationUrl.Should().Be("http://localhost:11434"); | ||
|
|
||
| // Act | ||
| var result = settings.Validate(); | ||
|
|
||
| // Assert | ||
| result.IsValid.Should().BeTrue(); | ||
| s.Provider = AIProvider.LMStudio; |
There was a problem hiding this comment.
[P1] ConfigurationUrl tests assume normalization that the property does not perform
The new ConfigurationUrl_Normalizes_For_Local_Providers asserts that assigning "localhost:11434" (or "127.0.0.1:1234") will automatically become http://…. However BrainarrSettings.ConfigurationUrl simply assigns to _ollamaUrl/_lmStudioUrl without passing through NormalizeHttpUrlOrOriginal, so the stored value remains as provided and the getter returns the un-normalized string. Without a matching production change these assertions will fail every run. Either call the normalization helper in the test (or in ConfigurationUrl) or relax the expectation to the raw value.
Useful? React with 👍 / 👎.
| [InlineData("example.com", false)] // no port or dot? Normalize requires dot or port; this has dot so inferred OK but IsValidUrl requires scheme; focus Normalize below | ||
| public void Missing_Scheme_Inferred_For_Local(string url, bool expected) | ||
| { | ||
| UrlValidator.IsValidUrl(url, allowEmpty: false).Should().Be(expected); |
There was a problem hiding this comment.
[P1] IsValidUrl test rejects example.com despite the implementation treating it as valid
In Missing_Scheme_Inferred_For_Local the case ("example.com", false) expects UrlValidator.IsValidUrl to return false when no scheme is supplied. The validator explicitly calls EnsureScheme, which prefixes http:// for strings containing a dot or port before validating via Uri.TryCreate, so example.com is accepted. This assertion will consistently fail unless the implementation is changed to forbid non-local hostnames without schemes. Adjust the expected value to true or validate with IsValidLocalProviderUrl if that is the desired constraint.
Useful? React with 👍 / 👎.
…ssertion; normalize ConfigurationUrl in BrainarrSettings
…ouching ConfigurationUrl setter
RicherTunes
changed the title
…HttpResponse> to avoid HttpException/StatusCode ambiguity in analysis build
RicherTunes
changed the title
…all property tests to new file
RicherTunes
force-pushed
the
main
branch
9 times, most recently
from
7aec68d to
024c063
Compare
|
Closing in favor of PR #146 which contains similar coverage improvements. Consolidating duplicate PRs. |
1 participant
Add targeted unit tests and code refinements to raise coverage and improve provider resilience:
All tests tagged Category=Unit. No production behavior changes.
Enhancements include:
This update refines and expands test coverage, improves test clarity and relevance, and enhances provider resilience handling.
🌿 Generated by Terry
ℹ️ Tag @terragon-labs to ask questions and address PR feedback
📎 Task: https://www.terragonlabs.com/task/e9bb08d9-44a7-4400-b3e7-2e4495f3e2ac