Catch: prominently display skipped tests #10669

Mytherin · 2024-02-14T15:40:44Z

This PR adds the SKIP_TEST command to Catch, and displays whether or not tests have been skipped. This should help prevent tests from being skipped accidentally, and allow us to more easily catch skipped tests in the CI.

All tests are skipped:

$ > build/reldebug/test/unittest test/sql/skiptest/test_skip.test 
[1/1] (100%): test/sql/skiptest/test_skip.test                                  
===============================================================================
All tests were skipped (total skipped 1)

Skipped tests for the following reasons:
require httpfs: 1

Tests are partially skipped:

$ > build/reldebug/test/unittest test/sql/skiptest/*
[2/2] (100%): test/sql/skiptest/test_not_skipped.test                           
===============================================================================
All tests passed (1 skipped test, 1 assertion in 1 test case)

Skipped tests for the following reasons:
require httpfs: 1

Fast unit tests on my macbook now:

$ > build/reldebug/test/unittest
[2891/2891] (100%): test/sql/table_function/icu_range_timestamptz.test          
===============================================================================
All tests passed (69 skipped tests, 381167 assertions in 2822 test cases)

Skipped tests for the following reasons:
require block_size: 3
require exact_vector_size: 1
require excel: 1
require fts: 6
require httpfs: 45
require longdouble: 2
require not_available: 1
require windows: 3
require-env LOCAL_EXTENSION_REPO: 7

run_tests_one_by_one.py

$ > python3 scripts/run_tests_one_by_one.py build/debug/test/unittest "test/sql/skiptest/*"          
[0/2]: test/sql/skiptest/test_skip.test (SKIPPED)
[1/2]: test/sql/skiptest/test_not_skipped.test (1 assertion )

carlopi

LGTM!

Disclaimer: I have not tried this locally (yet)

Tishj · 2024-02-14T16:21:05Z

I have tried this locally, seems to work, at least for a single test
The return code is 0, so it won't trigger a failure, which I think is correct

➜  duckdb git:(skippedtests) ✗ ./build/debug/test/unittest "test/tmp/tmp.test"
Filters: test/tmp/tmp.test
[1/1] (100%): test/tmp/tmp.test                                                 
===============================================================================
All tests were skipped (total skipped 1)

Skipped tests for the following reasons:
require httpfs: 1

With --require ... it will cause a return code 1

➜  duckdb git:(skippedtests) ✗ ./build/debug/test/unittest "test/tmp/tmp.test" --require httpfs
Filters: test/tmp/tmp.test
[0/1] (0%): test/tmp/tmp.test                                                   
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
unittest is a Catch v2.13.7 host application.
Run with -? for options

-------------------------------------------------------------------------------
test/tmp/tmp.test
-------------------------------------------------------------------------------
/Users/thijs/DuckDBLabs/duckdb/test/sqlite/test_sqllogictest.cpp:201
...............................................................................

/Users/thijs/DuckDBLabs/duckdb/test/sqlite/sqllogic_parser.cpp:115: FAILED:
explicitly with message:
  test/tmp/tmp.test:9: require httpfs: FAILED

carlopi · 2024-02-14T23:19:13Z

Tried locally, works nicely.

One potential improvement, add the number of skipped tests also on failed runs.
Currently on success it would be like:

===============================================================================
All tests passed (154 skipped tests, 371179 assertions in 2739 test cases)

Skipped tests for the following reasons:
require autocomplete: 1
require block_size: 3
require exact_vector_size: 1
require excel: 1
require fts: 5
...

while on failure:

===============================================================================
test cases:   689 |   688 passed | 1 failed
assertions: 96339 | 96338 passed | 1 failed

Skipped tests for the following reasons:
require not_available: 1
require-env LOCAL_EXTENSION_REPO: 7

maybe on failure there can be also the count of skipped, but that's minor.

Merge pull request duckdb/duckdb#10658 from hannes/csvpathlength Merge pull request duckdb/duckdb#10756 from Mytherin/preserveinsertionordermemory Merge pull request duckdb/duckdb#10746 from samansmink/enable-azure-autoload Merge pull request duckdb/duckdb#10747 from maiadegraaf/list_reverse_bug Merge pull request duckdb/duckdb#10748 from taniabogatsch/capi-tests Merge pull request duckdb/duckdb#10739 from peterboncz/pb/immmedate_mode_only_in_non_autocommit Merge pull request duckdb/duckdb#10688 from Tmonster/union_exclude Merge pull request duckdb/duckdb#10710 from samansmink/comment-on-column Merge pull request duckdb/duckdb#10725 from hawkfish/fuzzer-preceding-frame Merge pull request duckdb/duckdb#10723 from hawkfish/fuzzer-null-timestamp Merge pull request duckdb/duckdb#10436 from taniabogatsch/map-fixes Merge pull request duckdb/duckdb#10587 from kryonix/main Merge pull request duckdb/duckdb#10738 from TinyTinni/fix-assert-in-iscntrl Merge pull request duckdb/duckdb#10708 from carlopi/ci_fixes Merge pull request duckdb/duckdb#10726 from hawkfish/fuzzer-to-weeks Merge pull request duckdb/duckdb#10727 from hawkfish/fuzzer-window-bind Merge pull request duckdb/duckdb#10733 from TinyTinni/remove-static-string Merge pull request duckdb/duckdb#10715 from Tishj/python_tpch_regression_rework Merge pull request duckdb/duckdb#10728 from hawkfish/fuzzer-argminmax-decimal Merge pull request duckdb/duckdb#10717 from carlopi/fix_extension_deploy Merge pull request duckdb/duckdb#10694 from Mytherin/castquerylocation Merge pull request duckdb/duckdb#10448 from peteraisher/feature/use-assertThrows-for-jdbc-tests Merge pull request duckdb/duckdb#10691 from Mytherin/issue10685 Merge pull request duckdb/duckdb#10684 from Mytherin/distincton Merge pull request duckdb/duckdb#9539 from Tishj/timestamp_unit_to_tz Merge pull request duckdb/duckdb#10341 from Tmonster/tpch_ingestion_benchmark Merge pull request duckdb/duckdb#10689 from Mytherin/juliaversion Merge pull request duckdb/duckdb#10669 from Mytherin/skippedtests Merge pull request duckdb/duckdb#10679 from Tishj/reenable_window_rows_overflow Merge pull request duckdb/duckdb#10672 from carlopi/wasm_extensions_ci Merge pull request duckdb/duckdb#10660 from szarnyasg/update-storage-info-for-v0100 Merge pull request duckdb/duckdb#10643 from bleskes/duck_transaction_o11y Merge pull request duckdb/duckdb#10654 from carlopi/fix_10548 Merge pull request duckdb/duckdb#10650 from hannes/noprintf Merge pull request duckdb/duckdb#10649 from Mytherin/explicitenumnumbers

Mytherin added 3 commits February 13, 2024 13:24

Add SKIP_TEST catch command and keep track of number of skipped tests

c51cc75

Print list of tests that are skipped

91c65f0

Update run_tests_one_by_one

5bf0114

Mytherin requested a review from carlopi February 14, 2024 15:40

carlopi approved these changes Feb 14, 2024

View reviewed changes

Show skipped tests as well

f9fffcf

github-actions bot marked this pull request as draft February 15, 2024 08:38

Mytherin marked this pull request as ready for review February 15, 2024 08:51

Mytherin added the Ready To Merge label Feb 15, 2024

Mytherin merged commit 00c5d5b into duckdb:main Feb 15, 2024

Mytherin deleted the skippedtests branch July 5, 2024 11:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Catch: prominently display skipped tests #10669

Catch: prominently display skipped tests #10669

Uh oh!

Mytherin commented Feb 14, 2024

Uh oh!

carlopi left a comment

Uh oh!

Tishj commented Feb 14, 2024

Uh oh!

carlopi commented Feb 14, 2024

Uh oh!

Uh oh!

Catch: prominently display skipped tests #10669

Catch: prominently display skipped tests #10669

Uh oh!

Conversation

Mytherin commented Feb 14, 2024

Uh oh!

carlopi left a comment

Choose a reason for hiding this comment

Uh oh!

Tishj commented Feb 14, 2024

Uh oh!

carlopi commented Feb 14, 2024

Uh oh!

Uh oh!