Skip to content
Snippets Groups Projects

tests: Introduce custom test driver.

Merged Francisco Casas requested to merge fcasas/vkd3d:test_log_driver into master
4 unresolved threads

Basically !427 (closed), but in the form of a custom test driver script.

If I am not mistaken, for the driver script to run properly it is necessary to run "autoreconf" in the source directory and call the configure script again in the build directory.

The output when running tests would be the same we currently have except for tests that don't PASS, in that case, detailed information for each failing line will appear. Make a failing test to try it!

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
    • Author Developer

      Ugh, from the pipeline output I see that there are output differences in the other platforms. For the record a failing test looks like this for me:

      FAIL: tests/hlsl/nointerpolation.shader_test 
         (SM4.0-SM5.1)OpenGL.  165[XF] 
         (SM4.0-SM5.1)Vulkan.  165[XF] 
         (SM4.0-SM5.1)vkd3d.   165[XF] 
         (SM6.0-SM6.0)vkd3d.   147[F] 

      I may need to start looking at these one by one.

    • Author Developer

      So in the end the issue on mac was that I was using \S+ in the awk regexes and that is not POSIX portable, so I replaced it with [^[:space:]]+.

    • Please register or sign in to reply
  • Francisco Casas mentioned in merge request !427 (closed)

    mentioned in merge request !427 (closed)

    • +# Give detailed report
      +awk_program=$(cat <<'EOF'
      +BEGIN {
      +    FS = ":"
      +}
      +
      +/: Compiling \S+ shaders with \S+ and executing with \S+/ {
      +    n = split($3, arr, " ")
      +    str = "(" arr[2] ")"arr[n]
      +    printf("#   ^[[0;2m%-20s ^[[m\n", str)
      +}
      +
      +/: Test failed:/ {
      +    print $4 "^[[1;31m[F]^[[m"
      +}
      +
      +/: Todo:/ {
      +    print $4 "^[[2;32m[XF]^[[m"
      +}
      +
      +/: Todo succeeded:/ {
      +    print $4 "^[[1;31m[XF]^[[m"
      +}
      +
      +/: Assertion .* failed\./ {
      +    print "^[[1;31m[AF]^[[m" $0
      +}
      +
      +END {
      +    printf("#")
      +}
      +
      +EOF
      +)

      It seems somewhat unfortunate to always output colour; ideally we'd use $red, $green, and so on. The distinction between "Todo" and "Todo succeeded" seems a bit too subtle.

    • Author Developer

      Got it, I will find a way to use those variables. Also, I should have written [XP] for "Todo succeeded".

    • Please register or sign in to reply
    • Would it make sense to print XFAIL instead of PASS when there are expected failures inside a test case, even if the are no hard failures and therefore the full report is not emitted? It used to be that way before todo support was added in the shader runner.

    • Author Developer

      I tried it but it makes it complicated, at least for me, to visually parse red errors if the normal output becomes an amalgamation of "PASS" and "XFAIL" which have different length, and color.

      For that reason I propose the idea of handling these cases by keeping them as PASS but displaying the number of XF in parenthesis.

    • Yeah, it looks fine to me. The CI log is not colored, but I suppose that's because you haven't rebased on edc8e38b yet. Could you try to do that?

    • Author Developer

      Done!

    • Please register or sign in to reply
  • Francisco Casas added 1 commit

    added 1 commit

    • 6f8e3222 - tests: Get detailed information on failing tests.

    Compare with previous version

  • Francisco Casas added 36 commits

    added 36 commits

    • 6f8e3222...d65f331e - 34 commits from branch wine:master
    • 60267119 - tests: Copy autotool's testsuite driver script.
    • 712b6534 - tests: Get detailed information on failing tests.

    Compare with previous version

  • Francisco Casas added 1 commit

    added 1 commit

    • 199330eb - tests: Get detailed information on failing tests.

    Compare with previous version

  • Francisco Casas marked this merge request as ready

    marked this merge request as ready

  • I haven't reviewed the code, but I like the result, so I am approving.

  • Giovanni Mascellani approved this merge request

    approved this merge request

  • +dist_noinst_SCRIPTS = tests/test-driver.sh
    +dist_check_SCRIPTS = tests/test-driver.sh

    Do you need both of those? Wouldn't dist_check_SCRIPTS be sufficient on its own?

    +SHADER_TEST_LOG_DRIVER = tests/test-driver.sh

    We probably want that to be "SHADER_TEST_LOG_DRIVER = $(srcdir)/tests/test-driver.sh" for out-of-tree builds.

    +AM_COND_IF([BUILD_TESTS],
    +           [AC_CONFIG_LINKS([tests/test-driver.sh:tests/test-driver.sh])])

    ...and then we can just drop this.

  • Author Developer
    +dist_noinst_SCRIPTS = tests/test-driver.sh
    +dist_check_SCRIPTS = tests/test-driver.sh

    Do you need both of those? Wouldn't dist_check_SCRIPTS be sufficient on its own?

    I was wondering because I found this automake manual entry ambiguous:

    Scripts that need not be installed can be listed in noinst_SCRIPTS, and among them, those which are needed only by ‘make check’ should go in check_SCRIPTS. 
    - Section 9.1 Executable Scripts.

    But I see now that it also says:

    The special prefix ‘check_’ indicates that the objects in question should not be built until the ‘make check’ command is run. Those objects are not installed either.
    - Section 3.3 The Uniform Naming Scheme

    I also ran make install DESTDIR=... after removing dist_noinst_SCRIPTS and didn't see the script in the DESTDIR. So it is most probably not necessary.

    +SHADER_TEST_LOG_DRIVER = tests/test-driver.sh
    +AM_COND_IF([BUILD_TESTS],
    +           [AC_CONFIG_LINKS([tests/test-driver.sh:tests/test-driver.sh])])

    We probably want that to be "SHADER_TEST_LOG_DRIVER = $(srcdir)/tests/test-driver.sh" for out-of-tree builds.

    +AM_COND_IF([BUILD_TESTS],
    +           [AC_CONFIG_LINKS([tests/test-driver.sh:tests/test-driver.sh])])

    ...and then we can just drop this.

    Thanks! It looks far better than creating a symbolic link.

  • Francisco Casas added 2 commits

    added 2 commits

    • 7bb34c4a - tests: Copy autotool's testsuite driver script.
    • 0680240d - tests: Get detailed information on failing tests.

    Compare with previous version

  • Henri Verbeet approved this merge request

    approved this merge request

  • Alexandre Julliard approved this merge request

    approved this merge request

  • Alexandre Julliard added 43 commits

    added 43 commits

    • 0680240d...b13d60d8 - 41 commits from branch wine:master
    • faea8cd7 - tests: Copy autotool's testsuite driver script.
    • 9e9b3d26 - tests: Get detailed information on failing tests.

    Compare with previous version

Please register or sign in to reply
Loading