Skip to content

Let's rewrite the test harness#1394

Merged
StephanTLavavej merged 44 commits intomicrosoft:masterfrom
cbezault:master
Nov 6, 2020
Merged

Let's rewrite the test harness#1394
StephanTLavavej merged 44 commits intomicrosoft:masterfrom
cbezault:master

Conversation

@cbezault
Copy link
Contributor

@cbezault cbezault commented Oct 23, 2020 •
edited
Loading

This change is mostly cleanups, though it brings us more in line with libcxx's testing format in terms of style and functionality.

I also tried to decrease the size of the TestConfig and STLTest objects as much as possible since those are pickled and communicated to the worker processes via IPC. On my machine I saw a ~20% decrease in testing time with this change (~75mins to ~60mins), we'll see if that holds true on the test machines.

@cbezault cbezault added the test Related to test code label Oct 23, 2020
@cbezault cbezault self-assigned this Oct 23, 2020
cbezault
cbezault commented Oct 23, 2020
View reviewed changes
@cbezault cbezault changed the title test: let's rewrite the test harness Let's rewrite the test harness Oct 23, 2020
@cbezault cbezault requested a review from rpjohnst October 23, 2020 15:49
@cbezault
Copy link
Contributor Author

cbezault commented Oct 23, 2020

@rpjohnst If you could make sure I didn't break your edg_drop machinery that would be much appreciated.

@CaseyCarter CaseyCarter linked an issue Oct 23, 2020 that may be closed by this pull request
libc++ compile-only ".compile.pass.cpp" tests #1382
Closed
@cbezault cbezault marked this pull request as ready for review October 23, 2020 17:46
@cbezault cbezault requested a review from a team as a code owner October 23, 2020 17:46
cbezault and others added 2 commits November 2, 2020 17:10
@cbezault @StephanTLavavej
Update tests/utils/stl/test/format.py
54fa098 
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
@cbezault @StephanTLavavej
Update tests/utils/stl/test/tests.py
0a7194c 
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Nov 3, 2020 •
edited
Loading

I encountered one test failure because my repo root, S:\GitHub\STL, combined with an exceptionally long libcxx path, combined with /analyze's emission of a long-named XML file, exceeded the MAX_PATH limit. This made me realize that /analyze is emitting XML files that we never, ever want. I've pushed a commit to change all occurrences of plain /analyze to /analyze:autolog- which will still emit the warnings we want, but not the XML files. With this change, everything passes for x86:

Testing Time: 6095.88s
  Skipped          :   358
  Unsupported      :  1895
  Passed           : 22678
  Expectedly Failed:  1015

Same for x64:

Testing Time: 5919.71s
  Skipped          :   358
  Unsupported      :  3247
  Passed           : 21326
  Expectedly Failed:  1015

This doesn't change our /analyze:only options. I'll file an issue about that.

@StephanTLavavej StephanTLavavej mentioned this pull request Nov 3, 2020
Closed
@cbezault cbezault mentioned this pull request Nov 3, 2020
Closed
StephanTLavavej added a commit to StephanTLavavej/STL that referenced this pull request Nov 5, 2020
@StephanTLavavej
microsoftGH-1394 Let's rewrite the test harness
cc3c1a7
CaseyCarter
CaseyCarter suggested changes Nov 5, 2020
View reviewed changes
Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

I'm totally breaking @StephanTLavavej's 17-attack PR merge combo. 🐼

CaseyCarter
CaseyCarter suggested changes Nov 5, 2020
View reviewed changes
cbezault and others added 2 commits November 5, 2020 12:00
@cbezault @CaseyCarter
Update tests/utils/stl/test/tests.py
227efe0 
Co-authored-by: Casey Carter <cartec69@gmail.com>
@cbezault @CaseyCarter
Update tests/utils/stl/test/tests.py
3725d6d 
Co-authored-by: Casey Carter <cartec69@gmail.com>
CaseyCarter
CaseyCarter approved these changes Nov 5, 2020
View reviewed changes
Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

All good now!

StephanTLavavej added a commit to StephanTLavavej/STL that referenced this pull request Nov 5, 2020
@StephanTLavavej
microsoftGH-1394 Let's rewrite the test harness
84cc919
@StephanTLavavej StephanTLavavej merged commit 9ee5529 into microsoft:master Nov 6, 2020
@StephanTLavavej
Copy link
Member

StephanTLavavej commented Nov 6, 2020

Thanks for this overhaul, improving test performance! This will both increase maintainer/contributor productivity 🚀 and decrease our costs 🤑.

@StephanTLavavej StephanTLavavej mentioned this pull request Nov 6, 2020
Merged
@cbezault cbezault linked an issue Jul 14, 2021 that may be closed by this pull request
tests/libcxx: We're using the old libc++ test format which will be removed #922
Closed
@StephanTLavavej StephanTLavavej mentioned this pull request Jun 21, 2022
Merged
4 tasks
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

+3 more reviewers

@rpjohnst rpjohnst rpjohnst left review comments

@ben-craig ben-craig ben-craig left review comments

@CaseyCarter CaseyCarter CaseyCarter approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej StephanTLavavej

Labels

test Related to test code

Projects

None yet

Milestone

No milestone

5 participants

@cbezault @ben-craig @StephanTLavavej @rpjohnst @CaseyCarter