Skip to content

Add LIT parameter priority#5032

Merged
StephanTLavavej merged 4 commits intomicrosoft:mainfrom
CaseyCarter:test-prio
Oct 24, 2024
Merged

Add LIT parameter priority#5032
StephanTLavavej merged 4 commits intomicrosoft:mainfrom
CaseyCarter:test-prio

Conversation

@CaseyCarter
Copy link
Contributor

@CaseyCarter CaseyCarter commented Oct 23, 2024 •
edited
Loading

To control test process priority. Defaults to IDLE_PRIORITY_CLASS. Has no effect without pip install psutil.

@CaseyCarter
Add LIT parameter priority
ad59273 
To control test process priority. Defaults to `IDLE_PRIORITY_CLASS`.
@CaseyCarter CaseyCarter added the infrastructure Related to repository automation label Oct 23, 2024
@CaseyCarter CaseyCarter requested a review from a team as a code owner October 23, 2024 02:18
CaseyCarter
CaseyCarter commented Oct 23, 2024
View reviewed changes
@StephanTLavavej StephanTLavavej self-assigned this Oct 23, 2024
StephanTLavavej
StephanTLavavej reviewed Oct 23, 2024
View reviewed changes
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Oct 23, 2024

Thanks, this is awesome! 😻 Now test runs should interfere less with Microsoft Teams. 🐈‍⬛

I pushed a commit to print a note when psutil isn't installed. This looks like:

D:\GitHub\STL\out\x64>pip list
Package Version
------- -------
pip     24.2

D:\GitHub\STL\out\x64>python tests\utils\stl-lit\stl-lit.py %STL%\tests\std\tests\Dev11_0272959_make_signed --succinct
NOTE: Module "psutil" is not installed, so the priority setting "idle" has no effect.
-- Testing: 49 tests, 32 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..

Testing Time: 1.78s

Total Discovered Tests: 49
  Unsupported:  4 (8.16%)
  Passed     : 45 (91.84%)

D:\GitHub\STL\out\x64>python tests\utils\stl-lit\stl-lit.py %STL%\tests\std\tests\Dev11_0272959_make_signed --succinct -Dpriority=low
NOTE: Module "psutil" is not installed, so the priority setting "low" has no effect.
-- Testing: 49 tests, 32 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..

Testing Time: 1.78s

Total Discovered Tests: 49
  Unsupported:  4 (8.16%)
  Passed     : 45 (91.84%)

D:\GitHub\STL\out\x64>pip install psutil
Collecting psutil
  Downloading psutil-6.1.0-cp37-abi3-win_amd64.whl.metadata (23 kB)
Downloading psutil-6.1.0-cp37-abi3-win_amd64.whl (254 kB)
Installing collected packages: psutil
Successfully installed psutil-6.1.0

D:\GitHub\STL\out\x64>pip list
Package Version
------- -------
pip     24.2
psutil  6.1.0

D:\GitHub\STL\out\x64>python tests\utils\stl-lit\stl-lit.py %STL%\tests\std\tests\Dev11_0272959_make_signed --succinct
-- Testing: 49 tests, 32 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..

Testing Time: 1.81s

Total Discovered Tests: 49
  Unsupported:  4 (8.16%)
  Passed     : 45 (91.84%)

D:\GitHub\STL\out\x64>python tests\utils\stl-lit\stl-lit.py %STL%\tests\std\tests\Dev11_0272959_make_signed --succinct -Dpriority=low
-- Testing: 49 tests, 32 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..

Testing Time: 1.78s

Total Discovered Tests: 49
  Unsupported:  4 (8.16%)
  Passed     : 45 (91.84%)

@StephanTLavavej StephanTLavavej removed their assignment Oct 23, 2024
@StephanTLavavej StephanTLavavej mentioned this pull request Oct 23, 2024
Open
@StephanTLavavej StephanTLavavej added test Related to test code and removed infrastructure Related to repository automation labels Oct 23, 2024
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Oct 23, 2024

Relabeled as this affects local test runs, not repository automation. We actually no longer install psutil in the VM image, because our new security policies hate pip install and I don't know how to cromulently phrase it when preparing a non-corpnet image.

@CaseyCarter
Copy link
Contributor Author

CaseyCarter commented Oct 23, 2024

Thanks, this is awesome! 😻 Now test runs should interfere less with Microsoft Teams. 🐈‍⬛

Yeah, I had no doubt my motivation would be obvious to anyone who saw my slideshow in standup yesterday =)

I pushed a commit to print a note when psutil isn't installed.

I went back and forth on this a couple of times trying to avoid any visible message when the user didn't explicitly specify a value for the parameter. This turns out to be surprisingly complicated to do without adding a separate default to the set of acceptable values that LIT reports when you specify a value not in that set. Now it occurs to me that "default" (currently equivalent to "idle") wouldn't be terrible. I'll keep it in mind and we'll make a change if this message is too scary for new users.

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Oct 23, 2024

I think we do actually want a message when the default is used, since otherwise the unfriendly behavior is invisible. (What's missing is a way to suppress the message without installing psutil)

@StephanTLavavej StephanTLavavej self-assigned this Oct 23, 2024
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Oct 23, 2024

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 27973ad into microsoft:main Oct 24, 2024
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Oct 24, 2024

🐈‍⬛ 📹 📺

@CaseyCarter CaseyCarter deleted the test-prio branch October 24, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels

test Related to test code

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@CaseyCarter @StephanTLavavej