-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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.
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
feat(core): Add internal API for test definitions (no-changelog) #11591
feat(core): Add internal API for test definitions (no-changelog) #11591
Conversation
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
97404fd
to
0b0cde4
Compare
packages/cli/src/databases/repositories/test-definition.repository.ee.ts
Outdated
Show resolved
Hide resolved
packages/cli/src/databases/repositories/test-definition.repository.ee.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice and easy to follow code. Also good tests 👏 Few comments but nothing major, nice work 🥇
packages/cli/test/integration/evaluation/test-definitions.api.test.ts
Outdated
Show resolved
Hide resolved
// Fetch the first page | ||
let resp = await authOwnerAgent.get('/evaluation/test-definitions?take=10'); | ||
expect(resp.statusCode).toBe(200); | ||
expect(resp.body.data.count).toBe(15); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably also test that the returned test definitions are also correct (ids should be enough)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made an attempt, but could not quickly come up with a reliable way to add tests in a specific order (the retrieval is ordered by createdAt
field), so decided to postpone it
packages/cli/test/integration/evaluation/test-definitions.api.test.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Tomi Turtiainen <[email protected]>
Co-authored-by: Tomi Turtiainen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
✅ All Cypress E2E specs passed |
n8n
|
Project |
n8n
|
Branch Review |
ai-422-api-endpoint-evaluationtests-crud
|
Run status |
|
Run duration | 04m 22s |
Commit |
|
Committer | Eugene Molodkin |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
2
|
|
0
|
|
462
|
View all changes introduced in this branch ↗︎ |
Got released with |
Summary
This PR adds a set of internal API endpoints to work with Test Definitions.
Related Linear tickets, Github issues, and Community forum posts
Review / Merge checklist
release/backport
(if the PR is an urgent fix that needs to be backported)