dunst-project / dunst

Lightweight and customizable notification daemon
https://dunst-project.org
Other
4.64k stars 344 forks source link

test: use argument as test data directory #1373

Closed apprehensions closed 3 months ago

apprehensions commented 3 months ago

Required by #1226

codecov-commenter commented 3 months ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 65.34%. Comparing base (844167a) to head (b284582).

Files Patch % Lines
test/test.c 60.00% 2 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1373 +/- ## ========================================== - Coverage 65.37% 65.34% -0.03% ========================================== Files 50 50 Lines 8326 8326 Branches 1000 1001 +1 ========================================== - Hits 5443 5441 -2 - Misses 2883 2885 +2 ``` | [Flag](https://app.codecov.io/gh/dunst-project/dunst/pull/1373/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/dunst-project/dunst/pull/1373/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | `65.34% <60.00%> (-0.03%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

apprehensions commented 3 months ago

such a giant project with messy workflows, convuluted build instructions that are all over the place, and now this?

bynect commented 3 months ago

what was the problem?

apprehensions commented 3 months ago

It uses the directory the program is built in, which doesn't work for Meson.

bynect commented 3 months ago

since it is a simple change i'll go ahead and merge

zappolowski commented 2 months ago

@apprehensions, @bynect

FYI this broke parsing for greatest. Therefore, it's not possible anymore to execute specific tests only or ignore certain tests (among other features).

As greatest stops parsing at the first occurrence of -- one could use e.g.

diff --git a/Makefile b/Makefile
index f0d4a6f..b495960 100644
--- a/Makefile
+++ b/Makefile
@@ -94,7 +94,7 @@ endif
 test: test/test clean-coverage-run
    # Make sure an error code is returned when the test fails
    /usr/bin/env bash -c 'set -euo pipefail;\
-   ./test/test ./test | ./test/greenest.awk '
+   ./test/test -v -- ./test | ./test/greenest.awk '

 test-valgrind: test/test
    ${VALGRIND} \
@@ -104,7 +104,7 @@ test-valgrind: test/test
        --errors-for-leak-kinds=definite \
        --num-callers=40 \
        --error-exitcode=123 \
-       ./test/test ./test
+       ./test/test -v -- ./test

 test-coverage: CFLAGS += -fprofile-arcs -ftest-coverage -O0
 test-coverage: test
diff --git a/test/test.c b/test/test.c
index ab3927e..95f0e47 100644
--- a/test/test.c
+++ b/test/test.c
@@ -7,7 +7,7 @@

 #include "../src/log.h"
 #include "../src/settings.h"
-#include "helpers.h"
+#include "../src/utils.h"

 const char *base;

@@ -33,12 +33,19 @@ SUITE_EXTERN(suite_input);
 GREATEST_MAIN_DEFS();

 int main(int argc, char *argv[]) {
-        if (argc != 2) {
-                fprintf(stderr, "Usage: %s testdatadir", argv[0]);
+        int base_idx = -1;
+        for (int i = 1; i < argc - 1; ++i) {
+                if (STR_EQ(argv[i], "--") && i + 1 < argc) {
+                        base_idx = i + 1;
+                        break;
+                }
+        }
+        if (base_idx == -1) {
+                fprintf(stderr, "Usage: %s [GREATEST_OPTS] -- testdatadir\n", argv[0]);
                 exit(1);
         }

-        base = realpath(argv[1], NULL);
+        base = realpath(argv[base_idx], NULL);
         if (!base) {
                 fprintf(stderr, "Cannot determine actual path of test executable: %s\n", strerror(errno));
                 exit(1);

to restore the old behavior.

After having written this, I'm not sure whether this should be actually needed.

It uses the directory the program is built in, which doesn't work for Meson.

This just means, that the data needed for tests (the base configuration) should be put to the build directory as well.

apprehensions commented 2 months ago

-v isn't even a valid option, why would it be kept?

zappolowski commented 2 months ago

It is valid and it was used before ... and that's why it should be kept.

$ test/test -h
Usage: test/test [-hlfavex] [-s SUITE] [-t TEST] [-x EXCLUDE]
  -h, --help  print this Help
  -l          List suites and tests, then exit (dry run)
  -f          Stop runner after first failure
  -a          Abort on first failure (implies -f)
  -v          Verbose output
  -s SUITE    only run suites containing substring SUITE
  -t TEST     only run tests containing substring TEST
  -e          only run exact name match for -s or -t
  -x EXCLUDE  exclude tests containing substring EXCLUDE

run on 844167a (the latest commit on master before merging your branch).

bynect commented 2 months ago

Sorry I didn't even know those option existed since I always used the test as is 😬

Maybe we should use a DATA_DIR env then?

apprehensions commented 2 months ago

Or maybe switch to Meson tests :)

bynect commented 2 months ago

Or maybe switch to Meson tests :)

We can't switch overnight you know...