-
Notifications
You must be signed in to change notification settings - Fork 4.8k
TRT-2292: Add support for test isolation #30478
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
Open
xueqzhan
wants to merge
15
commits into
openshift:main
Choose a base branch
from
xueqzhan:test-isolation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,191
−38
Open
Changes from 13 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
a6a923f
Schedule tests with conflicts configured
xueqzhan 333855c
Fix some racing issues
xueqzhan 43f600d
Add test case for conflict tracker
xueqzhan 14295f8
Add support for taint and toleration
xueqzhan f264969
Rename conflictTracker to testScheduler
xueqzhan 6c929ff
Add conflict group to support instance and bucket mode
xueqzhan b2c029a
Implement testQueue to keep test order even with conflict handling
xueqzhan 53b61e9
Remove place holders for isolation mode handling
xueqzhan 020c59e
Address review comments
xueqzhan 745cf1d
Use taint and toleration from isolation struct
xueqzhan 4df7d1f
Simplify TestScheduler interface
xueqzhan 1c730e3
Code cleanup
xueqzhan df32893
Add context to GetNextTestToRun
xueqzhan d0e1d91
Address review comment about using spec
xueqzhan e61f83c
Add comment for conflict group
xueqzhan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
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.
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.
This needs to get replaced from the content in the spec right?
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.
spec doesn't define group. Group is designed to support mode.
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.
Sorry I'm not quite understanding what this function does.
Isolationhas two fields relevant,ConflictsandMode. I think we agreed to only supportmode=execfor now, but shouldn't we get theConflictsout of theIsolationstruct?Why does this always return
default?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.
The idea was to have a framework to support mode in the future. So conflict is only checked within a conflictGroup. Right now all tests belong to default group and therefore work like mode=exec. But just in case another mode is needed, more conflictGroup will be created for that purpose.
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.
Is there a reason not to call it mode here instead of conflict group, and use the name we're implementing ("exec")? It's not clear to me "group" is linked to the "mode"
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.
Mode is not clear to indicate a grouping mechanism. But conflictGroup is clear about its functionality of grouping. But configGroup can be used to implement mode. So I think conflictGroup is still better.