-
Notifications
You must be signed in to change notification settings - Fork 15
Feat: add test series field #1480
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
base: main
Are you sure you want to change the base?
Conversation
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.
Could you please rebase with my PR to get the build info for our TestSeriesView?
4c6938b
to
e7c535f
Compare
e7c535f
to
5166acf
Compare
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.
Pull Request Overview
This PR introduces a new series
field to the tests model for more efficient test identification and querying. The series field is a junction of path + platform and uses MD5 hashing to create unique identifiers. This allows for better handling of async insertion problems and provides a more efficient way to query test status history using series-based identification rather than complex joins.
Key changes include:
- Added a generated
series
field to the Tests model with MD5 hash of path + platform - Created a new TestStatusSeries endpoint mirroring TestStatusHistory functionality
- Fixed database router to prevent cache tables from being created on the default database
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
backend/schema.yml | Updated OpenAPI schema with new endpoint and parameter changes |
backend/kernelCI_app/models.py | Added generated series field with MD5 hash and database index |
backend/kernelCI_app/migrations/0004_add_test_series_field.py | Migration to add the series field to tests table |
backend/kernelCI_app/views/testStatusSeriesView.py | New view implementing TestStatusSeries endpoint |
backend/kernelCI_app/queries/test.py | Added get_series_data query function |
backend/kernelCI_app/helpers/test.py | Extracted test status history processing logic to shared helper |
backend/kernelCI_app/routers/databaseRouter.py | Fixed database router to exclude cache tables from default database |
backend/kernelCI_app/typeModels/testDetails.py | Added new request/response models and updated existing ones |
Comments suppressed due to low confidence (2)
backend/kernelCI_app/views/testStatusHistoryView.py:1
- The path field is annotated as
str
but has a default value ofNone
, which creates a type mismatch. It should beOptional[str]
to match the nullable default value.
from http import HTTPStatus
backend/kernelCI_app/queries/test.py:64
- Several parameters (path, git_repository_url, git_repository_branch, config_name) are typed as required
str
but the corresponding request model shows these can beNone
. The function signature should useOptional[str]
to match the API contract.
def get_test_status_history(
*,
path: str,
origin: Origin,
git_repository_url: str,
git_repository_branch: str,
platform: Optional[str],
test_start_time: Test__StartTime,
config_name: str,
field_timestamp: Timestamp,
group_size: int,
):
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
779e63f
to
b7c949e
Compare
b1b6b51
to
e9f3cb4
Compare
Fixes a problem where tests related to unexistent checkouts were not being able to return their history because of typing Closes #1167
Adds a test endpoint in order to compare with test status history endpoint, but using the new series fields
b7c949e
to
f43e712
Compare
Description
Adds a new field to the tests model called "series" which will be a junction of path + platform.
The approach is that we can create a "half-series" on the tests table, and then join on builds and use the similar build
series
field in order to identify a test. This is done specially so that we can avoid async insertion problems, when a test is inserted before its related build is inserted; it also allows us to use a generated column instead of a trigger and avoids having to query for the platform under a json field, with index.For testing, also adds a new endpoint called "TestStatusSeries" similar to TestStatusHistory, but using the series fields.
Note
Since TestStatusSeries and TestStatusHistory use different fields for filtering, their return is different. Not only that, but there is also a difference between filtering by checkout origin vs test origin specially for tests that are related to builds related to nonexistent checkouts. Example: http://localhost:5173/test/maestro:68ba7c39b9811ea53d062635
While I was checking the migrations, I also saw that the cache tables are being created on the postgres database, so I also fixed the database router while I was at it.
Also, when comparing TestStatusSeries with TestStatusHistory, I saw that tests related to nonexistent checkouts were simply not returning any history, so I fixed that too.
Changes
series
field to tests modelHow to test
Closes #1167
Closes #1475