tests: Add Claude agents for API testing#934
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #934 +/- ##
==========================================
- Coverage 51.48% 51.47% -0.01%
==========================================
Files 275 275
Lines 5988 5989 +1
Branches 1853 1853
==========================================
Hits 3083 3083
- Misses 2593 2594 +1
Partials 312 312
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Reviewer's GuideAdds four Claude agent configuration files that define an automated workflow for generating, reviewing, orchestrating, and analyzing Playwright API tests for Trustify UI, including detailed responsibilities, standards, and processes for each agent. Sequence diagram for orchestrated API test generation and analysissequenceDiagram
actor Developer
participant Orchestrator as api-test-orchestrator
participant Analyzer as api-coverage-analyzer
participant Generator as api-test-generator
participant Reviewer as api-test-reviewer
participant Spec as OpenAPI_trustd_yaml
participant Tests as Api_test_files
Developer ->> Orchestrator: Define testing goal
Orchestrator ->> Analyzer: Analyze API coverage
Analyzer ->> Spec: Read OpenAPI spec
Analyzer ->> Tests: Read existing API tests
Analyzer -->> Orchestrator: Coverage report + prioritized gaps
Orchestrator ->> Generator: Generate tests for top priority gaps
Generator ->> Spec: Read OpenAPI spec
Generator ->> Tests: Read existing tests for context
Generator -->> Developer: Draft Playwright API tests
Developer ->> Reviewer: Submit draft tests for review
Reviewer ->> Spec: Cross check against OpenAPI spec
Reviewer ->> Tests: Compare with existing patterns
Reviewer -->> Developer: Review comments and improvements
Developer ->> Tests: Integrate approved tests into suite
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- There’s a lot of duplicated guidance between the generator, reviewer, orchestrator, and coverage analyzer (e.g., URLSearchParams usage, Jira bugfix format, file locations); consider extracting shared standards into a single short reference section that each agent links to so future updates don’t get out of sync.
- Some of the embedded code examples (e.g.,
const fs = require('fs');, specificnpmcommands, and relative paths) assume particular module systems and project scripts—please align these snippets with the actual conventions and scripts used in the existing e2e tests so the agents don’t suggest patterns that won’t compile or run in this repo.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a lot of duplicated guidance between the generator, reviewer, orchestrator, and coverage analyzer (e.g., URLSearchParams usage, Jira bugfix format, file locations); consider extracting shared standards into a single short reference section that each agent links to so future updates don’t get out of sync.
- Some of the embedded code examples (e.g., `const fs = require('fs');`, specific `npm` commands, and relative paths) assume particular module systems and project scripts—please align these snippets with the actual conventions and scripts used in the existing e2e tests so the agents don’t suggest patterns that won’t compile or run in this repo.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
vobratil
left a comment
vobratil
left a comment
There was a problem hiding this comment.
@matejnesuta This is a pretty hefty PR, so I've only reviewed the api-test-standards section. I will try to continue tomorrow, but hopefully you can take a look at the comments already. I've never reviewed a PR of this type, so I'm not sure if my comments are useful. Any feedback would be welcome :)
| ### Rules | ||
|
|
||
| - **NEVER** import `test` or `expect` from `@playwright/test` directly — always use `"../fixtures"` | ||
| - Omit `expect` from the import if only `test` is needed |
There was a problem hiding this comment.
Maybe this should be a more general rule, something like "check if all imports are actually used in the code"?
There was a problem hiding this comment.
I wonder if our linter already does this. It should be definitely possible without an LLM.
|
|
||
| ```typescript | ||
| // ✓ Acceptable for simple static-only queries | ||
| const response = await axios.get("/api/v3/sbom?limit=10&offset=0"); |
There was a problem hiding this comment.
In addition to the URL formatting part, even though it may sound silly, I'm just wondering if it would make sense to keep all the .md files intended for AI to a minimum necessary length? The smaller the context, the smaller the token usage, I would assume, which could really add up on unnecessary parts over thousands of prompts. Maybe it's a small optimization, I don't know, but I think eventually AI pricing is going to become a concern.
There was a problem hiding this comment.
So, I understand that concern and I'll try to optimize the files once I address other issues in this PR. However, I am not really sure if it is going to change much in the grander scheme of things. I don't think we currently have any way of usage tracking of Claude across the whole Gaucsec. I have the same concern about the pricing, I just hope I won't spend way too much time on something, which did not need any optimalizations in the first place. 🥲
|
|
||
| ### Deep Object Matching | ||
|
|
||
| Use `toMatchObject` for deeply nested structures where the exact shape is important: |
There was a problem hiding this comment.
While this is good advice for a human, I'm not sure if the agent has much visibility into the test data / is capable of deciding when the shape is important. In my experience AI really struggles with this and more often than not this has to be done manually. Just something to consider - do we need this part for AI, or should we just ask it to use placeholders instead?
|
|
||
| ### File Upload Tests | ||
|
|
||
| Use `uploadFiles` from `general-helpers.ts` — never read files manually: |
There was a problem hiding this comment.
Theoretically the explicit use of uploadFiles should be saved only for tests when we, for whatever reason, don't want the files to be uploaded during the initial document ingestion, like when testing the latest endpoint, where the presence of certain SBOMs could make the test pointless.
However, this leads me to a semi-related thought that we currently have no proper cleanup in our tests, except for a handful of my tests, where it was necessary, otherwise the tests would break after the first run. Perhaps that's something we could mention here? E.g. that the env should be brought back to the original state at the end of each test run.
There was a problem hiding this comment.
"we currently have no proper cleanup in our tests"
I think this is kind of tied to a question, whether we need one in our tests or not. Back when I was in Service Mesh, we did not have any clean-ups in our tests. Instead, there was a some sort of "Before" condition, which prepared the environment for the test execution. This was done to make finding root cause of bugs easier in case some of the tests failed in our Jenkins pipelines.
Maybe this is something to be discussed with the whole team.
| - Always `await` axios calls | ||
| - No floating promises | ||
|
|
||
| ### No Hard-Coded Waits |
There was a problem hiding this comment.
There's actually a few cases where this may be valid, e.g. in the case of Atlas tests, which uses specific timeouts. Maybe we could just say that timeouts should be parameterizable in the test header?
There was a problem hiding this comment.
Do you maybe have an example for this? I could not find anything in the code-base myself
2 participants
Summary by Sourcery
Add Claude agent configurations to support automated generation, review, orchestration, and coverage analysis for Playwright-based API tests in Trustify UI.
New Features: