Kernel mode test harness by cbezault · Pull Request #1421 · microsoft/STL

cbezault · 2020-11-03T21:19:31Z

Merge after #1394

Fix tpyos

We correctly deprecated `atomic_init` when implementing P0883R2, but missed that the effects changed as well.

Handle calling convention differences

Always compile kernel tests, but only run on x64

Manually resolved: azure-devops/run-build.yml

StephanTLavavej

Quick partial review.

StephanTLavavej · 2021-02-14T02:49:03Z

tests/std/tests/Dev11_0863628_atomic_compare_exchange/test.cpp

+        }                                           \
+    }
+
+#else


Suggested change

#else

#else // _KERNEL_MODE

StephanTLavavej · 2021-02-14T02:49:11Z

tests/std/tests/Dev11_0863628_atomic_compare_exchange/test.cpp

        abort();
    }
 }
+#endif


Suggested change

#endif

#endif // _KERNEL_MODE

StephanTLavavej · 2021-02-14T02:50:01Z

tests/utils/kernel/generateMsvcCert.ps1

@@ -0,0 +1,26 @@
+param([string]$out="MsvcStlTestingCert.pfx",[string]$pass="foo")


New file needs a copyright/license banner.

StephanTLavavej · 2021-02-14T02:50:19Z

tests/utils/kernel/generateMsvcCert.ps1

+Where-Object { $_.Subject -eq 'CN=MsvcStlTestingCert' } |
+Remove-Item
+
+#Make the new cert


Suggested change

#Make the new cert

# Make the new cert

StephanTLavavej · 2021-02-14T02:51:53Z

tests/utils/kernel/generateMsvcCert.ps1

@@ -0,0 +1,26 @@
+param([string]$out="MsvcStlTestingCert.pfx",[string]$pass="foo")


We conventionally avoid "foo" as a placeholder. Can we use something else here?

StephanTLavavej · 2021-02-14T02:54:52Z

tests/utils/kernel/inc/stl_kernel/stl_kernel.h

+typedef struct TestResults {
+    int main_return;
+    long tests_failed;
+    char output[4088];


Is this number magical or arbitrary?

StephanTLavavej · 2021-02-14T02:56:44Z

tests/utils/kernel/src/stl_kernel_loader/testapp.cpp

+struct DeviceOpener {
+    HANDLE h = INVALID_HANDLE_VALUE;
+    explicit DeviceOpener(const char* driverName) {
+        std::string dosName = "\\\\.\\";


Raw string literals would make this easier to read.

StephanTLavavej · 2021-02-14T02:57:26Z

tests/utils/kernel/src/stl_kernel_loader/testapp.cpp

+    char InputBuffer[1];
+    TestResults OutputBuffer;
+
+    auto success = DeviceIoControl(dev.h, (DWORD) IOCTL_SIOCTL_METHOD_RUN_TEST, &InputBuffer, sizeof(InputBuffer),


We conventionally avoid C-style casts.

StephanTLavavej · 2021-02-14T03:00:55Z

azure-devops/native-build-test.yml

    parallelismFlag: '-j$(testParallelism)'
    xmlOutputFlag: '--xunit-xml-output=$(${{ parameters.buildOutputLocationVar }})/test-results.xml'
-    shardFlags: '--num-shards=$(System.TotalJobsInPhase);--run-shard=$(System.JobPositionInPhase)'
+    shardFlags: '-vv;--num-shards=$(System.TotalJobsInPhase);--run-shard=$(System.JobPositionInPhase)'


Is -vv necessary/useful, or is it a relic of development?

I think this one is useful. Without this, CI failures tell you which tests failed, but give very little output from the failing test. But it's hard for me to verify that since this is in the CI machinery.

I'd say this is useful since upstream broke our ability to write nice output to the xml test results.

StephanTLavavej · 2021-02-14T03:01:20Z

azure-devops/cmake-configure-build.yml

    -DCMAKE_BUILD_TYPE=Release ^
    -DLIT_FLAGS=$(litFlags) ^
-    -DCMAKE_CXX_FLAGS=/analyze:autolog- ^
+    -DCMAKE_CXX_FLAGS="/showIncludes /analyze:autolog-" ^


Is /showIncludes a relic of development? I am concerned about the amount of log spam it could generate.

StephanTLavavej

This has a merge conflict with main now - can you resolve it?

ben-craig · 2021-04-29T01:15:05Z

I haven't forgotten about this... it's just that the machine that I'd been using to work on this died, and with the chip shortage, I've been having a hard time getting a replacement. I do plan on applying all the changes that have been suggested.

Kernel5

cbezault · 2021-06-30T02:03:14Z

stl/inc/ranges

        };

        inline constexpr _Drop_fn drop;
+


Looks like some ranges changes snuck in by accident?

ben-craig · 2021-07-04T21:08:28Z

Superseded by #2040

CaseyCarter and others added 30 commits July 25, 2020 20:43

Update LLVM reference

7515060

Merge branch 'master' of https://github.com/microsoft/STL into HEAD

46f7b77

Avoid relying on upstream libcxx lit.cfg

d4ff68b

Skip bogus failing tests

f62ad74

Apply suggestions from code review

7a859cd

Fix tpyos

Update LLVM reference to latest

777d49b

Update atomic_init per P0883R3

5d3ee9a

We correctly deprecated `atomic_init` when implementing P0883R2, but missed that the effects changed as well.

Update LLVM reference once again

e46a162

Fix mis-formatted line in expected_results.txt

40b5aac

Fix bad separators in libcxx/expected_results

16c00b2

Let's rewrite the test harness

c72c9fd

Fix merge conflicts

0a379b4

Get past file validation

8b3708f

That's what I get for not testing x86

352715d

Undo unwanted changes to submodules fixup tests which embed /c flag

9bb228a

resolve merge conflict

428a2fc

Resolves microsoftGH-1382

0b5cdaf

Merge branch 'master' of https://github.com/Microsoft/STL

1843c30

cleanup the new custom format

1d3cc67

Fix broken custom format

d6eab20

Respect tests getting marked as unsupported

f5d374c

Fix .fail tests

c332008

STL_EDG_DROP at least does something now + other cleanups

d0cb477

Make handling the intellisense response file its own step type

9f400b0

Make all compile-only tests compile only

dd9a2a2

Actually find tests with new suffix

331017e

Update format.py

74660fd

Merge with master

5116e2c

Merge branch 'master' of github.com:cbezault/STL

82e8f41

Fix a bug that STL found

be7e4a4

cbezault and others added 7 commits December 21, 2020 14:38

Update tests.py

f041be3

Merge pull request #11 from ben-craig/kernel3

593f49a

Handle calling convention differences

clang-format

6f78416

Always compile kernel tests, but only run on amd64

db798ef

Update format.py

3886633

Merge pull request #12 from ben-craig/kernel4

bd9f7e4

Always compile kernel tests, but only run on x64

Update format.py

ba553ad

StephanTLavavej mentioned this pull request Jan 21, 2021

Toolset update: VS 2019 16.9 Preview 3 #1577

Merged

This comment has been minimized.

Sign in to view

Base automatically changed from master to main January 28, 2021 00:35

cbezault marked this pull request as ready for review February 13, 2021 19:03

cbezault requested a review from a team as a code owner February 13, 2021 19:03

StephanTLavavej added 2 commits February 13, 2021 16:29

Merge branch 'main' into kernel

e31ce62

Manually resolved: azure-devops/run-build.yml

Define CMAKE_CXX_FLAGS and STL_IS_KERNEL on separate lines.

99e3d46

StephanTLavavej reviewed Feb 14, 2021

View reviewed changes

StephanTLavavej requested changes Apr 28, 2021

View reviewed changes

ben-craig and others added 4 commits May 16, 2021 09:11

Merge branch 'main' into local_kernel

aca1692

STL review comments

f44db64

Merge branch 'master' into kernel5

1454c77

Merge pull request #13 from ben-craig/kernel5

054d7fc

Kernel5

cbezault commented Jun 30, 2021

View reviewed changes

stl/inc/ranges

};

inline constexpr _Drop_fn drop;

Copy link

Contributor Author

cbezault Jun 30, 2021

Choose a reason for hiding this comment

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

Looks like some ranges changes snuck in by accident?

ben-craig mentioned this pull request Jul 4, 2021

Kernel mode test harness #2040

Closed

StephanTLavavej closed this Jul 6, 2021

+                      }                                           \
+                  }
+              #else

		@@ -0,0 +1,26 @@
		param([string]$out="MsvcStlTestingCert.pfx",[string]$pass="foo")

Conversation

cbezault commented Nov 3, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

StephanTLavavej left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StephanTLavavej left a comment

Choose a reason for hiding this comment

Uh oh!

ben-craig commented Apr 29, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ben-craig commented Jul 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants