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

Add LIT parameter priority #5032

Merged
merged 4 commits into from
Oct 24, 2024
Merged

Conversation

CaseyCarter
Copy link
Member

@CaseyCarter CaseyCarter commented Oct 23, 2024

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

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
@StephanTLavavej StephanTLavavej self-assigned this Oct 23, 2024
@StephanTLavavej
Copy link
Member

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 added test Related to test code and removed infrastructure Related to repository automation labels Oct 23, 2024
@StephanTLavavej
Copy link
Member

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
Member Author

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

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

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
39 checks passed
@StephanTLavavej
Copy link
Member

🐈‍⬛ 📹 📺

@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
Labels
test Related to test code
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants