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

significantly increase test coverage #3290

Merged
merged 18 commits into from
Sep 23, 2019
Merged

significantly increase test coverage #3290

merged 18 commits into from
Sep 23, 2019

Conversation

andrewrk
Copy link
Member

  • add zig build option -Dskip-libc to skip tests that build libc
    (e.g. if you don't want to wait for musl to build)
  • add -Denable-wine option which uses wine to run cross compiled
    windows tests on non-windows hosts
  • add -Denable-qemu option which uses qemu to run cross compiled
    foreign architecture tests
  • add -Denable-foreign-glibc=path option which combined with
    -Denable-qemu enables running cross compiled tests that link
    against glibc. See
    https://github.com/ziglang/zig/wiki/Updating-libc#glibc for how to
    produce this directory.
  • the test matrix is done manually. release test builds are only
    enabled by default for the native target. this should save us some CI
    time, while still providing decent coverage of release builds.
  • enable qemu testing on the Linux CI job. There's not really a good
    reason to enable wine, since we have a Windows CI job as well.
  • remove the no longer needed --build-file ../build.zig from CI
    scripts
  • fix bug in glibc compilation where it wasn't properly reading the abi
    list txt files, resulting in "key not found" error.
  • std.build.Target gains:
    • isNetBSD
    • isLinux
    • osRequiresLibC
    • getArchPtrBitWidth
    • getExternalExecutor
  • zig build system gains support for enabling wine and enabling qemu.
    artifact.enable_wine = true;, artifact.enable_qemu = true;. This
    communicates that the system has these tools installed and the build
    system will use them to run tests.
  • zig build system gains support for overriding the dynamic linker of
    an executable artifact.
  • fix std.c.lseek prototype. makes behavior tests for
    arm-linux-musleabihf pass.
  • disable std lib tests that are failing on ARM. See std lib tests failing on aarch64 #3288, std lib tests failing on arm 32-bit #3289
  • provide std.os.off_t.
  • disable some of the compiler_rt symbols for arm 32 bit. Fixes
    compiler_rt tests for arm 32 bit
  • add __stack_chk_guard when linking against glibc. Fixes std lib tests
    for aarch64-linux-gnu
  • workaround for "unable to inline function" using @inlineCall. Fixes
    compiler_rt tests for arm 32 bit.

return self.isDarwin() or self.isFreeBSD() or self.isNetBSD();
}

pub fn getArchPtrBitWidth(self: Target) u32 {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is right and wrong at the same time, the effective pointer size is given by the architecture and the ABI: x32 means the full 64bit registers are used but the pointer size if trimmed to 4 bytes instead of 8.

@mikdusan
Copy link
Member

#3292 might fix freebsd error: unknown argument: '-faddrsig'

Copy link
Contributor

@euantorano euantorano 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 not sure disabling this check for S_ISCHR is the best way to fix an issue on a particular platform. This check is common practice when dealing with /dev/urandom (I've included a list below of some examples of other languages and libraries making use of it for example). I'm going to do some testing on some different platforms so I can see what's going on.

std/os.zig Outdated
@@ -145,11 +145,6 @@ fn getRandomBytesDevURandom(buf: []u8) !void {
const fd = try openC(c"/dev/urandom", O_RDONLY | O_CLOEXEC, 0);
defer close(fd);

const st = try fstat(fd);
Copy link
Contributor

Choose a reason for hiding this comment

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

I've just spun up an aarch64 system on Scaleway:

Linux test-aarch64-urandom 4.9.93-mainline-rev1 #1 SMP Tue Apr 10 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux

Using file /dev/urandom returns:

/dev/urandom: character special (1/9)

Which is as expected.

I have then spun up an AMD64 system and ran an aarch64 image in QEMU using these instructions (though using Ubuntu 18.04 instead of 16.04):

Linux ubuntu 4.15.0-64-generic #73-Ubuntu SMP Thu Sep 12 13:18:31 UTC 2019 aarch64 aarch64 aarch64 GNU/Linux

Connecting to this and running file /dev/urandom results in:

/dev/urandom: character special (1/9)

As expected.

I'm not sure exactly what the build/test script is doing to test the aarch64 stuff, but it must be doing something weird for /dev/urandom to not be a character device...

Copy link
Contributor

Choose a reason for hiding this comment

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

I have then spun up an AMD64 system and ran an aarch64 image in QEMU using these instructions (though using Ubuntu 18.04 instead of 16.04):

The tests are done using QEMU's userspace emulation instead of the classic full-system approach. All you have to do is cross-compile the test exercising getRandomBytesDevURandom and run it with qemu-aarch64.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll give that a go and see what happens then, thanks.

 * add zig build option `-Dskip-libc` to skip tests that build libc
   (e.g. if you don't want to wait for musl to build)
 * add `-Denable-wine` option which uses wine to run cross compiled
   windows tests on non-windows hosts
 * add `-Denable-qemu` option which uses qemu to run cross compiled
   foreign architecture tests
 * add `-Denable-foreign-glibc=path` option which combined with
   `-Denable-qemu` enables running cross compiled tests that link
   against glibc. See
   https://github.com/ziglang/zig/wiki/Updating-libc#glibc for how to
   produce this directory.
 * the test matrix is done manually. release test builds are only
   enabled by default for the native target. this should save us some CI
   time, while still providing decent coverage of release builds.
   - add test coverage for `x86_64-linux-musl -lc` (building musl libc)
   - add test coverage for `x86_64-linux-gnu -lc` (building glibc)
   - add test coverage for `aarch64v8_5a-linux-none`
   - add test coverage for `aarch64v8_5a-linux-musl -lc` (building musl libc)
   - add test coverage for `aarch64v8_5a-linux-gnu -lc` (building glibc)
   - add test coverage for `arm-linux-none`
   - test coverage for `arm-linux-musleabihf -lc` (building musl libc) is
     disabled due to #3286
   - test coverage for `arm-linux-gnueabihf -lc` (building glibc) is disabled
     due to #3287
   - test coverage for `x86_64-windows-gnu -lc` (building mingw-w64) is
     disabled due to #3285
 * enable qemu testing on the Linux CI job. There's not really a good
   reason to enable wine, since we have a Windows CI job as well.
 * remove the no longer needed `--build-file ../build.zig` from CI
   scripts
 * fix bug in glibc compilation where it wasn't properly reading the abi
   list txt files, resulting in "key not found" error.
 * std.build.Target gains:
   - isNetBSD
   - isLinux
   - osRequiresLibC
   - getArchPtrBitWidth
   - getExternalExecutor
 * zig build system gains support for enabling wine and enabling qemu.
   `artifact.enable_wine = true;`, `artifact.enable_qemu = true;`. This
   communicates that the system has these tools installed and the build
   system will use them to run tests.
 * zig build system gains support for overriding the dynamic linker of
   an executable artifact.
 * fix std.c.lseek prototype. makes behavior tests for
   arm-linux-musleabihf pass.
 * disable std lib tests that are failing on ARM. See #3288, #3289
 * provide `std.os.off_t`.
 * disable some of the compiler_rt symbols for arm 32 bit. Fixes
   compiler_rt tests for arm 32 bit
 * add __stack_chk_guard when linking against glibc. Fixes std lib tests
   for aarch64-linux-gnu
 * workaround for "unable to inline function" using `@inlineCall`. Fixes
   compiler_rt tests for arm 32 bit.
The ABI was incorrect.
After upgrading to LLVM 9 we now have the emscripten OS available. Fix
the compile error in this function by specifying the default ABI for
emscripten.
It was using __GNUC__ < 8 to check if _xgetbv intrinsic is available.
Since we always use zig cc with this header, we know we have clang 9,
which does have this intrinsic.
Previously the CMAKE_BUILD_TYPE=Release was conflicting with the
"distribution" directory `release`. I renamed this to `dist` so that
it won't conflict with any build types.
@andrewrk andrewrk merged commit 35c1d8c into master Sep 23, 2019
@andrewrk andrewrk deleted the more-test-coverage branch September 23, 2019 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants