Skip to content

Commit 1ffda75

Browse files
authored
[tests] Fix test semantics, JniValueMarshalerContractTests (#474)
Commit ffbb2dc introduced a unit test failure, as it overlooked updating an "expected" string within the test `Java.InteropTests.JniValueMarshaler_IJavaPeerable_ContractTests.CreateReturnValueFromManagedExpression()`. The more important question is this: why was that test failure missed? Turns Out™ that our unit test infrastructure is "dodgy": it was possible for tests to fail while the overall `make run-all-tests` target reported success! For example, as of commit ffbb2dc we *know* that `Java.Interop-Tests.dll` will report a test failure. Yet, if we run `make run-tests` such that a different (successful) unit test assembly follows it, no error is reported: $ make run-tests TESTS="bin/TestDebug/Java.Interop-Tests.dll bin/TestDebug/Java.Interop.Export-Tests.dll" ... …/Java.Interop/build-tools/scripts/RunNUnitTests.targets(35,5): error MSB3073: The command "mono --debug packages/NUnit.ConsoleRunner.3.9.0/tools/nunit3-console.exe bin/TestDebug/Java.Interop-Tests.dll --result="TestResult-Java.Interop-Tests.xml;format=nunit2" --output="bin/TestDebug/TestOutput-Java.Interop-Tests.txt"" exited with code 1. 0 Warning(s) 1 Error(s) ... $ echo $? 0 *This* is why the [DevOps build for PR #472][0] was green and the PR was merged. It's also why one of the [subsequent master builds][1] was green, even though it [reported unit test failures][2]. Our tests are unreliable. What will we do about it? 1. Update `make run-all-tests` to explicitly invoke targets. 2. Update the other `run-*` targets to exit with a non-zero value if *any* command exits with a non-zero value. `make run-all-tests` cannot use prerequisites, because once a prerequisites fails, all following prerequisites are skipped, and we want *all* of the unit tests to run, so that we collect as many errors as possible from a test run. As such, this rule: run-all-tests: run-tests run-ptests must be converted into: run-all-tests: r=0; \ $(MAKE) run-tests || r=1; \ $(MAKE) run-ptests || r=1; \ exit $$r The other `run-*` targets must be fixed in a similar way, to ensure that if *any* command fails, the entire rule *also* fails, while also executing as many tests as possible. With these changes in place, `make run-all-tests` will now report an error if any of the executed tests fail: $ make run-tests TESTS="bin/TestDebug/Java.Interop-Tests.dll bin/TestDebug/Java.Interop.Export-Tests.dll" ... $ echo $? 2 Finally, fix the error reported from `Java.InteropTests.JniValueMarshaler_IJavaPeerable_ContractTests`. [0]: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=2950259&view=results [1]: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=2950430&view=results [2]: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=2950430&view=ms.vss-test-web.build-test-results-tab
1 parent 5fe28cd commit 1ffda75

File tree

2 files changed

+15
-5
lines changed

2 files changed

+15
-5
lines changed

Makefile

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,13 @@ BUILD_PROPS = bin/Build$(CONFIGURATION)/JdkInfo.props bin/Build$(CONFIGURATION)/
4646

4747
all: $(DEPENDENCIES) $(TESTS)
4848

49-
run-all-tests: run-tests run-test-jnimarshal run-test-generator-core run-ptests
49+
run-all-tests:
50+
r=0; \
51+
$(MAKE) run-tests || r=1 ; \
52+
$(MAKE) run-test-jnimarshal || r=1 ; \
53+
$(MAKE) run-test-generator-core || r=1 ; \
54+
$(MAKE) run-ptests || r=1 ; \
55+
exit $$r;
5056

5157
include build-tools/scripts/msbuild.mk
5258

@@ -145,14 +151,18 @@ shell:
145151

146152
# $(call RUN_TEST,filename,log-lref?)
147153
define RUN_TEST
148-
$(MSBUILD) $(MSBUILD_FLAGS) build-tools/scripts/RunNUnitTests.targets /p:TestAssembly=$(1) ;
154+
$(MSBUILD) $(MSBUILD_FLAGS) build-tools/scripts/RunNUnitTests.targets /p:TestAssembly=$(1) || r=1;
149155
endef
150156

151157
run-tests: $(TESTS) bin/Test$(CONFIGURATION)/$(JAVA_INTEROP_LIB)
152-
$(foreach t,$(TESTS), $(call RUN_TEST,$(t),1))
158+
r=0; \
159+
$(foreach t,$(TESTS), $(call RUN_TEST,$(t),1)) \
160+
exit $$r;
153161

154162
run-ptests: $(PTESTS) bin/Test$(CONFIGURATION)/$(JAVA_INTEROP_LIB)
155-
$(foreach t,$(PTESTS), $(call RUN_TEST,$(t)))
163+
r=0; \
164+
$(foreach t,$(PTESTS), $(call RUN_TEST,$(t))) \
165+
exit $$r;
156166

157167
bin/Test$(CONFIGURATION)/$(JAVA_INTEROP_LIB): bin/$(CONFIGURATION)/$(JAVA_INTEROP_LIB)
158168
cp $< $@

src/Java.Interop/Tests/Java.Interop/JniValueMarshalerContractTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ protected override string GetExpectedReturnValueFromManagedExpression (string jv
601601
}}
602602
else
603603
{{
604-
return {value}_ref = {value}.PeerReference;
604+
return {value}_ref = (IJavaPeerable){value}.PeerReference;
605605
}}
606606
{value}_rtn = References.NewReturnToJniRef({value}_ref);
607607
return {pret.Name};

0 commit comments

Comments
 (0)