From 5e4196cabe1e1898188288e14d59351b9dfebf4e Mon Sep 17 00:00:00 2001 From: Rene Schneider Date: Thu, 23 Aug 2018 12:35:42 +0200 Subject: [PATCH 1/5] Fixed issue #196 by creating fake results in any abort case on a fork Fork result summaries (= the aggregate numbers for an execution block on a fork) used to not be transmitted explicitly, but were scraped out of the normal set list result transmissions by the master. However, this creates some problems with detecting abort execution exceptions, which are rooted in the fact that the master cannot easily discern between sub-suite results returned by the fork and the "top-level" suite result, because those look identical to the master. Therefore it would be cleaner to explicitly return suite result numbers via a separate dedicated remoting message, thus in turn solving all these problems caused by the old approach. This however requires a change to the remoting protocol, which is also used between test runners and the Eclipse plugin. Such changes are not desired for patch versions, but only for major and minor versions, which is why issue #196 is now fixed "twice": once on 0.18.0 in the "clean" way by adding this new message, and once on 0.17.14 by just removing a condition around a block on the master that creates a "fake" result summary for a fork in case of an abortion happening on the fork. The latter also fixes the issue #196 by guaranteeing the report of at least one exception if an abort execution exception is encountered, but it results in possibly wrong result numbers being displayed on the console on the master. --- .../integrity/runner/DefaultTestRunner.java | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java b/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java index f0ddb3aaf..e3efb668c 100644 --- a/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java +++ b/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java @@ -831,8 +831,10 @@ protected void initializeParameterizedConstants() { String tempValue = (parameterizedConstantValues != null) ? parameterizedConstantValues.get(tempName) : null; try { - defineConstant(tempDefinition, tempValue, (tempDefinition.eContainer() instanceof SuiteDefinition) - ? ((SuiteDefinition) tempDefinition.eContainer()) : null); + defineConstant(tempDefinition, tempValue, + (tempDefinition.eContainer() instanceof SuiteDefinition) + ? ((SuiteDefinition) tempDefinition.eContainer()) + : null); } catch (ClassNotFoundException | InstantiationException | UnexecutableException exc) { // Cannot happen - parameterized constants aren't evaluated } @@ -1182,19 +1184,17 @@ protected SuiteSummaryResult callSuiteSingle(Suite aSuiteCall) { // TODO make this nicer, it's kind of ugly to create a fake object with null values abortExecutionCause = new ExceptionWrapper(null, null); - if (tempResult == null && tempForkResultSummary == null) { - // We may not have any result at this point, as the fork has aborted without providing us - // one. In order to ensure that at least the exception that triggered the abortion is logged - // in the counts (and thus a user who typically only looks at the total counts is alerted to - // the problem), we generate a result here with one exception. This might be wrong actually - // (for example there could have been a successful test before the exception, which may not - // be counted in this case), but that's basically what's meant by "test result total numbers - // may be inaccurate" which is printed out in this case, and the current structure of the - // master-fork sync protocol makes it hard to perfectly fix this inaccuracy in cases of - // sudden execution path deviations. Thus, this "forced result" was deemed a good-enough - // solution. This fixes issue #145: https://github.com/integrity-tf/integrity/issues/145 - tempResult = new SuiteSummaryResult(0, 0, 1, 0, tempSuiteDuration); - } + // We may not have any result at this point, as the fork has aborted without providing us + // one. In order to ensure that at least the exception that triggered the abortion is logged + // in the counts (and thus a user who typically only looks at the total counts is alerted to + // the problem), we generate a result here with one exception. This might be wrong actually + // (for example there could have been a successful test before the exception, which may not + // be counted in this case), but that's basically what's meant by "test result total numbers + // may be inaccurate" which is printed out in this case, and the current structure of the + // master-fork sync protocol makes it hard to perfectly fix this inaccuracy in cases of + // sudden execution path deviations. Thus, this "forced result" was deemed a good-enough + // solution. This fixes issue #145: https://github.com/integrity-tf/integrity/issues/145 + tempResult = new SuiteSummaryResult(0, 0, 1, 0, tempSuiteDuration); } // and afterwards we'll switch back to real test mode @@ -1528,7 +1528,8 @@ protected void defineConstant(final ConstantDefinition aDefinition, Object aValu protected void defineVariable(final VariableOrConstantEntity anEntity, Object anInitialValue, final SuiteDefinition aSuite) { final Object tempInitialValue = (anInitialValue instanceof Variable) - ? variableManager.get(((Variable) anInitialValue).getName()) : anInitialValue; + ? variableManager.get(((Variable) anInitialValue).getName()) + : anInitialValue; // We need to send variable updates to forks in the main phase here. boolean tempSendToForks = (!isFork()) && shouldExecuteFixtures(); From d29e62e49e7fdc5da08ea540870a816c27c9c013 Mon Sep 17 00:00:00 2001 From: Rene Schneider Date: Thu, 6 Sep 2018 17:30:46 +0200 Subject: [PATCH 2/5] fixed issue #197: Exception on assign with call-assigned variables --- .../de/gebit/integrity/runner/DefaultTestRunner.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java b/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java index e3efb668c..61078b295 100644 --- a/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java +++ b/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java @@ -1732,7 +1732,17 @@ protected void executeVariableAssignment(VariableAssignment anAssignment, SuiteD // run or test run mode. try { setVariableValueConverted(anAssignment.getTarget().getName(), anAssignment.getValue(), true); - } catch (InstantiationException | ClassNotFoundException | UnexecutableException exc) { + } catch (UnexecutableException exc) { + // This is expected to happen in some cases during dry run, namely on operations to be executed on + // assignment which use variables that are dynamically assigned from call results. But that is not a + // problem, we can safely ignore this, the variable will then not be assigned. In the real test run + // however it should not happen, so let's print it on stderr in that case. + // This fixes issue #197: Exception thrown before test runs in case of assigns having operations + // with variables filled via call (https://github.com/integrity-tf/integrity/issues/197) + if (shouldExecuteFixtures()) { + exc.printStackTrace(); + } + } catch (InstantiationException | ClassNotFoundException exc) { exc.printStackTrace(); } } From bd6debe5fe50d03b1126d3a07af8025f5767af9c Mon Sep 17 00:00:00 2001 From: Rene Schneider Date: Tue, 11 Sep 2018 09:29:37 +0200 Subject: [PATCH 3/5] fixed issue #198: UnexecutableException on fork bc of lazy constant rslv --- .../de/gebit/integrity/runner/DefaultTestRunner.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java b/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java index 61078b295..edbb31d70 100644 --- a/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java +++ b/de.gebit.integrity.runner/src/de/gebit/integrity/runner/DefaultTestRunner.java @@ -1508,7 +1508,17 @@ protected void defineConstant(final ConstantDefinition aDefinition, Object aValu throws ClassNotFoundException, InstantiationException, UnexecutableException { Object tempValue; if (aValue == null) { - tempValue = parameterResolver.resolveStatically(aDefinition, variantInExecution); + if (isFork() && !shouldExecuteFixtures()) { + // Skip the value evaluation if we are on a fork AND the fork is not currently executing stuff. + // This complements a similar piece of code seen in the variable definition code that prevents + // the variable/constant to be defined if we're on a fork and the fork is in dry run mode. + // Values that are thrown away later anyway don't need to be computed, and if the are, this can + // even cause exceptions, because some input for those values might not be defined, as seen in + // issue #198, which this exception here intends to fix. + tempValue = null; + } else { + tempValue = parameterResolver.resolveStatically(aDefinition, variantInExecution); + } } else { tempValue = aValue; } From 28fbb8aa42af7963256232cb406d53acbf982452 Mon Sep 17 00:00:00 2001 From: Rene Schneider Date: Thu, 27 Sep 2018 15:07:56 +0200 Subject: [PATCH 4/5] fixed #199: Array element type mismatch possible with array objects --- .../parameter/conversion/AbstractModularValueConverter.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/de.gebit.integrity.dsl/src/de/gebit/integrity/parameter/conversion/AbstractModularValueConverter.java b/de.gebit.integrity.dsl/src/de/gebit/integrity/parameter/conversion/AbstractModularValueConverter.java index da86598ec..89841de2f 100644 --- a/de.gebit.integrity.dsl/src/de/gebit/integrity/parameter/conversion/AbstractModularValueConverter.java +++ b/de.gebit.integrity.dsl/src/de/gebit/integrity/parameter/conversion/AbstractModularValueConverter.java @@ -516,8 +516,7 @@ protected Object convertEncapsulatedValueToTargetType(Class aTargetType, Clas } else if (tempResult != null && tempResult.getClass().isArray()) { // A slight variant of the collection case is an array (possibly filled with Integrity types) // We just convert the individual elements in this case. - Object tempResultArray = Array.newInstance(tempResult.getClass().getComponentType(), - Array.getLength(tempResult)); + Object tempResultArray = Array.newInstance(aTargetType, Array.getLength(tempResult)); for (int i = 0; i < Array.getLength(tempResult); i++) { Array.set(tempResultArray, i, convertSingleValueToTargetType(aTargetType, aParameterizedType, Array.get(tempResult, i), aConversionContext, someVisitedValues)); From 5399c9d4de0a090e257faeed79b7433a18ea0d85 Mon Sep 17 00:00:00 2001 From: Rene Schneider Date: Thu, 27 Sep 2018 15:08:35 +0200 Subject: [PATCH 5/5] added regression test for issue #199 --- .../integrity/fixtures/NoOpFixture.integrity | 2 ++ .../suites/basic/simpleFixtureCalls.integrity | 10 +++++++ .../tests/junit/basic/SimpleFixtureCalls.java | 15 ++++++++++ ...ty.basic.simpleArrayCreationAndPassing.xml | 28 +++++++++++++++++++ .../tests/fixtures/basic/NoOpFixture.java | 9 ++++++ 5 files changed, 64 insertions(+) create mode 100644 de.gebit.integrity.tests/results/integrity.basic.simpleArrayCreationAndPassing.xml diff --git a/de.gebit.integrity.tests/integrity/fixtures/NoOpFixture.integrity b/de.gebit.integrity.tests/integrity/fixtures/NoOpFixture.integrity index 562f2341e..984dbbe2b 100644 --- a/de.gebit.integrity.tests/integrity/fixtures/NoOpFixture.integrity +++ b/de.gebit.integrity.tests/integrity/fixtures/NoOpFixture.integrity @@ -94,5 +94,7 @@ packagedef integrity.fixtures.basic.noop with testdef throwAbortExceptionTest uses de.gebit.integrity.tests.fixtures.basic.NoOpFixture#throwAbortException calldef generateCharacter uses de.gebit.integrity.tests.fixtures.basic.NoOpFixture#generateCharacter + + calldef generateArray uses de.gebit.integrity.tests.fixtures.basic.NoOpFixture#generateArray packageend \ No newline at end of file diff --git a/de.gebit.integrity.tests/integrity/suites/basic/simpleFixtureCalls.integrity b/de.gebit.integrity.tests/integrity/suites/basic/simpleFixtureCalls.integrity index 7b2877beb..b6c7d9764 100644 --- a/de.gebit.integrity.tests/integrity/suites/basic/simpleFixtureCalls.integrity +++ b/de.gebit.integrity.tests/integrity/suites/basic/simpleFixtureCalls.integrity @@ -67,6 +67,16 @@ packagedef integrity.basic with suiteend + + suitedef simpleArrayCreationAndPassing with + + // This failed because of issue 199 + variable someVariable + call integrity.fixtures.basic.noop.generateArray -> someVariable + call integrity.fixtures.basic.noop.echoObject echo: someVariable + + suiteend + packageend \ No newline at end of file diff --git a/de.gebit.integrity.tests/junit/de/gebit/integrity/tests/junit/basic/SimpleFixtureCalls.java b/de.gebit.integrity.tests/junit/de/gebit/integrity/tests/junit/basic/SimpleFixtureCalls.java index 638815628..4fe950962 100644 --- a/de.gebit.integrity.tests/junit/de/gebit/integrity/tests/junit/basic/SimpleFixtureCalls.java +++ b/de.gebit.integrity.tests/junit/de/gebit/integrity/tests/junit/basic/SimpleFixtureCalls.java @@ -85,4 +85,19 @@ public void testSimpleFixtureCallWithTwoParams() throws ModelLoadException, IOEx assertDocumentMatchesReference(tempResult); } + /** + * Performs a suite which does tests and checks the resulting XML document. + * + * @throws ModelLoadException + * @throws IOException + * @throws JDOMException + */ + @Test + public void testSimpleArrayCreationAndPassing() throws ModelLoadException, IOException, JDOMException { + Document tempResult = executeIntegritySuite( + new String[] { "integrity/suites/basic/simpleFixtureCalls.integrity" }, + "integrity.basic.simpleArrayCreationAndPassing", null); + assertDocumentMatchesReference(tempResult); + } + } diff --git a/de.gebit.integrity.tests/results/integrity.basic.simpleArrayCreationAndPassing.xml b/de.gebit.integrity.tests/results/integrity.basic.simpleArrayCreationAndPassing.xml new file mode 100644 index 000000000..b4387343a --- /dev/null +++ b/de.gebit.integrity.tests/results/integrity.basic.simpleArrayCreationAndPassing.xml @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/de.gebit.integrity.tests/src/de/gebit/integrity/tests/fixtures/basic/NoOpFixture.java b/de.gebit.integrity.tests/src/de/gebit/integrity/tests/fixtures/basic/NoOpFixture.java index 6a025d97d..b46398b52 100644 --- a/de.gebit.integrity.tests/src/de/gebit/integrity/tests/fixtures/basic/NoOpFixture.java +++ b/de.gebit.integrity.tests/src/de/gebit/integrity/tests/fixtures/basic/NoOpFixture.java @@ -226,6 +226,15 @@ public String generateCharacter(@FixtureParameter(name = "code") int aCharacterC return tempChar; } + @FixtureMethod(description = "Creates an array") + public Long[] generateArray() { + Long[] tempTestArray = new Long[5]; + for (int i = 0; i < 5; i++) { + tempTestArray[i] = new Long(i); + } + return tempTestArray; + } + public enum Enum { VALUE1,