Running tests with MinGW build

[braydonf]

I tried running tests with wine with the make check build, and ran into some issues.

With these changes it worked:
https://gist.github.com/braydonf/41091d8a63d111aa2d954dccb1ad0d56

[hawicz]

MinGW doesn't have an "unsigned long" type, or it has some kind of issues with the %lx format? Well, that's not really a valid set of changes to include in the main sources.
Unconditionally defining ETXTBSY and ENOTBLK isn't valid either.
I applied the change to switch from strndup to strdup, but that's about all I can include. (commit 75825a9)

[braydonf]

There are also a lot of warnings that will trigger failure because of -Werror

What are the recommendations for fixing the other issues? Considering that it's very close, seems to be worth fixing.

[hawicz]

Putting the E... defines inside ifndef's with a comment explaining why that's there could be reasonable.

What's the actual problem with the %lx format?

Warnings should be fixed, not ignored. What does it complain about?

[braydonf]

Running ./configure --host=x86_64-w64-mingw32 --prefix=/some/path and make check produces with -Werror:

make[2]: Entering directory '/home/bgf/vendor/json-c/tests'
  CC       test_util_file.o
test_util_file.c: In function ‘test_read_nonexistant’:
test_util_file.c:151:20: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
          filename, (unsigned long)jso);
                    ^
test_util_file.c: In function ‘test_read_closed’:
test_util_file.c:182:10: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
          (unsigned long) jso);
          ^
test_util_file.c:190:9: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
         (unsigned long)jso, json_util_get_last_err());
         ^
cc1: all warnings being treated as errors

[ploxiln]

So those would have to be %llx conversions and (unsigned long long) casts on windows (which would work on 64-bit unix, but give a similar warning on 32-bit unix). %p should do the right thing in all cases, it's for pointers.

[hawicz]

Well, the thought was that %p isn't defined to have a consistent representation across all platforms, but trying to emit pointer values and expecting consistency even across multiple runs of the same compiled binary on the same system is a lost cause. So, you're right, those uses should be changed to %p, since they're just error cases anyway.

[braydonf]

With current master branch and this patch https://gist.github.com/braydonf/1428d5d91c08f5c381d4e6cb83529484 tests will compile with ./configure --host=x86_64-w64-mingw32 --disable-shared --prefix=/some/path

I've just realized there are several bash scripts in the tests that may be causing issue when using wine. However running each .exe individually will pass, though this is likely not running the full test suite? What is the argument that is needed for test_util_file.exe?

[hawicz]

You need to pass it the location of the input files, i.e. valid.json which is in the tests directory. If you're in the tests directory while running it then it would simply be something like:

./test_util_file.exe .

Of course, if you don't run the test_util_file.test wrapper script then you'll just get the raw output without having it compared to the expected output in test_util_file.expected, so to actually verify things you'll need to do the diff yourself.

[hawicz]

hmm... some comments about the mingw format issue:
http://stackoverflow.com/questions/14071713/what-is-wrong-with-printfllx
http://stackoverflow.com/questions/13590735/printf-long-long-int-in-c-with-gcc

[hawicz]

I'll apply those changes, since we happen to know that the values in question here are within range. However, that's not really a proper fix, and you shouldn't expect building on mingw to continue to work in general.

[braydonf]

Okay. So I've been able to have the tests run with wine via make check USE_VALGRIND=0 by modifying test-defs.sh (this could be done via environment variable also), and removing \r before comparing the results.

---
 tests/test-defs.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/test-defs.sh b/tests/test-defs.sh
index 5a32d39..817904e 100755
--- a/tests/test-defs.sh
+++ b/tests/test-defs.sh
@@ -98,7 +98,7 @@ run_output_test()
 		err=$?
 
 	else
-		eval "\"${top_builddir}/${TEST_COMMAND}"\" \"\$@\" ${REDIR_OUTPUT}
+		eval wine "\"${top_builddir}/${TEST_COMMAND}"\" \"\$@\" ${REDIR_OUTPUT}
 		err=$?
 	fi
 
@@ -114,6 +114,8 @@ run_output_test()
 		fi
 	fi
 
+	sed -i 's/\r$//' "${TEST_OUTPUT}.out"
+
 	if ! "$CMP" -s "${srcdir}/${TEST_OUTPUT}.expected" "${TEST_OUTPUT}.out" ; then
 		echo "ERROR: \"${TEST_COMMAND} $@\" (${TEST_OUTPUT}) failed (set VERBOSE=1 to see full output):" 1>&2
 		(cd "${CURDIR}" ; set -x ; diff "${srcdir}/${TEST_OUTPUT}.expected" "$testsubdir/${TEST_OUTPUT}.out")
-- 
2.7.4

With results:

FAIL: test_util_file.test
PASS: test_float.test
PASS: test1.test
PASS: test2.test
PASS: test4.test
PASS: testReplaceExisting.test
FAIL: test_parse_int64.test
PASS: test_null.test
PASS: test_cast.test
PASS: test_double_serializer.test
PASS: test_parse.test
PASS: test_locale.test
PASS: test_charcase.test
PASS: test_printbuf.test
PASS: test_set_serializer.test
PASS: test_compare.test
PASS: test_set_value.test
PASS: test_visit.test

Not sure what's going on with the other tests yet.

[braydonf]

you shouldn't expect building on mingw to continue to work in general

Why is that? Adding it to the CI should help maintain that it continues to work.

[hawicz]

Since it's not an environment that I consider a "first class" target for json-c, I'm reluctant to impose restrictions on what kind of code may be included just to allow it to work within mingw, and I'm unlikely to require that proposed changes keep it working (or even notice when they break).
However, if you (or anyone else) are willing to figure out minimally invasive fixes and submit PRs for them, I'll be happy to merge them.