feat(BA-1561): Add session status history retriever test #4697
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seedspirit
commented
Jun 13, 2025
src/ai/backend/test/testcases/session/status_history_retriever.py
Outdated
Show resolved
Hide resolved
seedspirit
force-pushed
the
feat/add-session-status-history-retrieve-test
branch
from
60c5a9f to
7926f57
Compare
Comment on lines
15
to
38
| expected_status = set([ | ||
| "PENDING", | ||
| "SCHEDULED", | ||
| "PREPARING", | ||
| "PULLING", | ||
| "PREPARED", | ||
| "CREATING", | ||
| "RUNNING", | ||
| "RESTARTING", | ||
| "RUNNING_DEGRADED", | ||
| "TERMINATING", | ||
| "TERMINATED", | ||
| "ERROR", | ||
| "CANCELED", | ||
| ]) | ||
|
|
||
| result = await session.ComputeSession(name=session_name).get_status_history() | ||
| assert result["result"] != "", "Status history should not be empty" | ||
| # NOTE: SessionStatus enum is not available due to BUILD rules | ||
| # NOTE: src/ai/backend/test/BUILD[//src/ai/backend/**] -> src/ai/backend/manager/BUILD[!*] : DENY | ||
| for status_key, status_value in result["result"].items(): | ||
| assert status_key in expected_status, ( | ||
| f"Status {status_key} is not a valid session status" | ||
| ) |
There was a problem hiding this comment.
What kind of test are you trying to do?
There was a problem hiding this comment.
I was trying to test the session status history retrieval API is working properly and second, none of the retrieved status histories have unexpected session history values
There was a problem hiding this comment.
Please create tests for this part like, "In both success and failure scenarios, a specific history must have existed." I don't think it's necessary to verify parts that are already checked by type checking with test code again.
There was a problem hiding this comment.
As get_status_history only retrieve activate session history, I only defined test code with success scenario
seedspirit
force-pushed
the
feat/add-session-status-history-retrieve-test
branch
2 times, most recently
from
ee49c4a to
2cfcd00
Compare
HyeockJinKim
approved these changes
Jun 18, 2025
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
seedspirit
force-pushed
the
feat/add-session-status-history-retrieve-test
branch
from
2cfcd00 to
2e71ffe
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
seedspirit
force-pushed
the
feat/add-session-status-history-retrieve-test
branch
from
2e71ffe to
813aa96
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to a conflict with the base branch
seedspirit
force-pushed
the
feat/add-session-status-history-retrieve-test
branch
from
16b0e79 to
0765177
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #4682 (BA-1561)