Run PAL tests in CI by davidwrighton · Pull Request #42049 · dotnet/runtime

davidwrighton · 2020-09-10T00:04:59Z

Refactor the PAL test framework to be useable in our CI system, and enable it for testing

The existing PAL tests are problematic in a few ways for our CI.

They are large in terms of disk space used per test, this interferes with building them properly, especially as they are part of the product build, and not of the test build
While part of the product build, the option to build them was well hidden.
The tests are not related to our existing tests, and in particular are not driven from managed xunit wrappers.

The change here has 4 components

Reduce the volume of the binaries such that they can be passed from a product build to a test job effectively. This is done by merging all tests which are not strictly dependent on secondary libraries.

This was done via a tool which parsed and replaced all of the CMakeFiles.txt
And then manual editing to remove all duplicate global symbols

The build flag for building the tests has been exposed as a first class option to build-runtime, as well as through the subset mechanism. As of this change, developers who wish to build the tests should use the clr.paltests subset to build the tests.
As the tests do not have normal xunit wrappers, but they do have a script which will generate xunit output, they cannot quite be run in the existing helix pathway. Add a separate path for launch the appropriate helix workitem as part of outerloop test runs.
A new issue exclusion mechanism has been built. Add exclusions by modifying src/coreclr/src/pal/tests/palsuite/issues.targets

There are a number of failures in the current test suite that should probably be investigated, but making meaningful changes to the tests should not be part of this change.

davidwrighton · 2020-09-10T00:27:13Z

@janvorli In particular, when you review I'd like to know how difficult I've made new platform bringup. I think its still reasonable, but it isn't quite the same as before.

am11 · 2020-09-10T03:40:47Z

Why are these code changes necessary? e.g. ~9 days ago (#41622), master branch was building with ./src/coreclr/build-runtime.sh -skipgenerateversion -nopgooptimize -cmakeargs -DCLR_CMAKE_BUILD_TESTS=1 on Ubuntu, macOS and OpenIndiana hosts.

With this change nearly everything is building into 1 large binary. These symbols were interfering with linking, and while they could be fixed by making them static, I've always found statics to be annoying to debug, so I didn't use static.

janvorli

I can see an error message after the PAL tests successfully complete:

Finished running PAL tests.

PAL Test Results:
  Passed: 709
  Failed: 0

src/pal/tests/palsuite/runpaltests.sh: line 230: cd: /tmp/PalTestOutput/default/eventprovidertest: No such file or directory

As for new platform bringup, this change is fine. All we need for bringup is the ability to build and run the tests without any managed components being involved and this change preserves that.

janvorli · 2020-09-10T09:36:11Z

The oneValueArg should now be removed here since it doesn't exist anymore.

janvorli · 2020-09-10T10:32:57Z

A micro nit - maybe use pushd / popd instead?

janvorli · 2020-09-10T10:37:23Z

It was a bit unexpected that the path I needed to pass to this script was not the root of the build, but rather its paltests subfolder. I would prefer changing it so that we pass in the build root on the command line (e.g. ....../artifacts/bin/coreclr/Linux.x64.Debug) and add the "paltests" in this script to it.

By making the required parameter to being the paltests directory, it is now easy to dump these tests into our CI system. I do not intend to change this.

Just being curious - why would it be a problem for the CI to pass in a path without the palsuite subfolder and adding it in the script?

am11 · 2020-09-12T03:38:12Z

nit:

Suggested change

BUILD_ROOT_DIR=$1

BUILD_ROOD_DIR=$1

if [ -d "$(pwd)/$BUILD_ROOT_DIR" ]; then

BUILD_ROOT_DIR="$(pwd)/$BUILD_ROOT_DIR"

fi

currently it does not work with the relative paths:

# does not work $ src/coreclr/src/pal/tests/palsuite/runpaltests.sh artifacts//bin/coreclr/OSX.x64.Debug/paltests # works $ src/coreclr/src/pal/tests/palsuite/runpaltests.sh $(pwd)/artifacts//bin/coreclr/OSX.x64.Debug/paltests

both cases will work with the suggested change.

Also, now it is a directory instead of path to txt file, could you please update this doc section:

runtime/docs/workflow/testing/coreclr/unix-test-instructions.md

Lines 91 to 93 in 9e09fe3

```sh

./src/coreclr/src/pal/tests/palsuite/runpaltests.sh $(pwd)/artifacts/obj/coreclr/$(uname).x64.Debug

```

to:

```sh src/coreclr/src/pal/tests/palsuite/runpaltests.sh artifacts//bin/coreclr/$(uname).x64.Debug/paltests # on macOS, replace $(uname) with OSX ```

Remove directories for non-existent CreateDirectoryW test Disable build of LoadLibraryA test6 Disable build of LoadLibraryA test8 Remove CharNextA and CharNextExA infra Add palsuite.h to child process Common paltests code Fix issues disable tests

am11 · 2020-09-14T14:22:38Z

Sorry about the conflicts, we cleaned up some unused PAL API. 😊

…t build to install to bin directory, and be runnable from thereInitial infra to launch pal tests in helix as part of normal test runFix issue where clr buidl subsets not involving runtime do not work on Unix buildsPAL test integration with non-innerloop coreclr test runsFix linker issue when compiling on CentOS 6Try againFix build so it stops at a good place when it fails.Spelling errorFix additional destinations processing in jit

janvorli

LGTM, thank you!

Dotnet-GitSync-Bot added the area-PAL-coreclr only for closed issues label Sep 10, 2020

davidwrighton requested a review from janvorli September 10, 2020 00:18

davidwrighton linked an issue Sep 10, 2020 that may be closed by this pull request

Run PAL tests in CI #34776

Closed

am11 reviewed Sep 10, 2020

View reviewed changes

janvorli reviewed Sep 10, 2020

View reviewed changes

davidwrighton force-pushed the paltests branch 2 times, most recently from cba53dd to 4495b3f Compare September 12, 2020 01:19

am11 reviewed Sep 12, 2020

View reviewed changes

davidwrighton added 2 commits September 13, 2020 16:38

Automated changes

429ecff

davidwrighton added 4 commits September 14, 2020 10:39

I missed a semicolon, causing really weird failures in CI

be81d25

Code review feedback

2a299e2

Fix incorrect use of PAL_Terminate

6f5cf58

am11 mentioned this pull request Sep 14, 2020

Delete unused multibyte methods from PAL #42157

Merged

davidwrighton added 2 commits September 14, 2020 10:48

Remove deleted tests

037e468

Code review feedback

bf2ef58

davidwrighton force-pushed the paltests branch from 4495b3f to bf2ef58 Compare September 14, 2020 17:54

davidwrighton added 3 commits September 14, 2020 17:34

Attempt to fix both permissions and launch issues for the helix command

1d99b50

Rejigger how payload tar is arranged

4d6c725

Adjust xunit xml output to match with the Helix xunit parser

7511083

davidwrighton mentioned this pull request Sep 15, 2020

Helix SDK should support artifact directories with unix permissions dotnet/arcade#6183

Closed

1 task

davidwrighton added 2 commits September 15, 2020 19:18

Add fine grained support for disabling pal tests in CI

25eda67

Update test exclusion to match actual failures

3bde7f0

davidwrighton marked this pull request as ready for review September 16, 2020 18:31

janvorli approved these changes Sep 16, 2020

View reviewed changes

Update test running instructions

8dede62

davidwrighton merged commit 0c36a9a into dotnet:master Sep 16, 2020

ghost locked as resolved and limited conversation to collaborators Dec 7, 2020

davidwrighton deleted the paltests branch April 20, 2021 17:44

-BUILD_ROOT_DIR=$1
+BUILD_ROOD_DIR=$1
+if [ -d "$(pwd)/$BUILD_ROOT_DIR" ]; then
+  BUILD_ROOT_DIR="$(pwd)/$BUILD_ROOT_DIR"
+fi

	```sh
	./src/coreclr/src/pal/tests/palsuite/runpaltests.sh $(pwd)/artifacts/obj/coreclr/$(uname).x64.Debug
	```

Conversation

davidwrighton commented Sep 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidwrighton commented Sep 10, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janvorli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

am11 commented Sep 14, 2020

Uh oh!

janvorli left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

davidwrighton commented Sep 10, 2020 •

edited

Loading