-
Notifications
You must be signed in to change notification settings - Fork 350
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
test: use argument as test data directory #1373
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
such a giant project with messy workflows, convuluted build instructions that are all over the place, and now this? |
what was the problem? |
It uses the directory the program is built in, which doesn't work for Meson. |
since it is a simple change i'll go ahead and merge |
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 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.
This just means, that the data needed for tests (the base configuration) should be put to the build directory as well. |
-v isn't even a valid option, why would it be kept? |
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). |
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? |
Or maybe switch to Meson tests :) |
We can't switch overnight you know... |
Required by #1226