Skip to content
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

Rerun artefact test executions #173

Merged
merged 3 commits into from
May 8, 2024
Merged

Conversation

omar-selo
Copy link
Collaborator

This is the backend side of https://warthogs.atlassian.net/browse/RTW-307

Changes:

  • Adds a new endpoint POST /v1/artefacts/<artefact-id>/reruns that allows user to rerun all test executions belonging to an artefact
  • Adds some useful filters to the above endpoint

Note that CI will fail until we merge and rebase #172

@omar-selo omar-selo force-pushed the rerun-artefact-test-executions branch from 7df6328 to 5b66de7 Compare May 7, 2024 13:30
Base automatically changed from keep-review-after-rerun to main May 7, 2024 13:43
@omar-selo omar-selo force-pushed the rerun-artefact-test-executions branch from 5b66de7 to a2823a4 Compare May 7, 2024 13:48
Copy link
Contributor

@andrejvelichkovski andrejvelichkovski left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, just one concern about the review decision filter

RerunArtefactTestExecutionsRequest,
)

router = APIRouter(tags=["artefacts"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding the tag here 😄


class RerunArtefactTestExecutionsRequest(BaseModel):
test_execution_status: TestExecutionStatus | None = None
test_execution_review_decision: list[TestExecutionReviewDecision] | None = None
Copy link
Contributor

Choose a reason for hiding this comment

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

The review_decision is defined as a set in the TestExecutionDTO, although here it wouldn't really change a lot, if you want we can make it a set

test_executions = (te for te in test_executions if te.status == status)
if (decision := request.test_execution_review_decision) is not None:
test_executions = (
te for te in test_executions if te.review_decision == decision
Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't define the review decision as a set maybe here we should do set comparison instead of list as the order doesn't really matter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah very good point

Copy link
Contributor

@andrejvelichkovski andrejvelichkovski left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for addressing the comment!

@omar-selo omar-selo merged commit 5cc1685 into main May 8, 2024
2 of 3 checks passed
@omar-selo omar-selo deleted the rerun-artefact-test-executions branch May 8, 2024 08:36
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