Fixed test step result sync, made it independent from updating test issues

[martingrossmann]

Description

Syncing test results to test steps is now independent from creating or updating Xray test issues.

If test steps defined in your Testerra test, Xray connector try to sync the results to existing test issue steps in Xray.

Using the DefaultSummaryMapper means that Xray test issues are updated. Also existing test steps will be overwritten by Testerra test steps.

Fixes #16

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[Zsar]

It would be better to concatenate these into one log statement together with the very plain "Error at syncing with Jira", such that said statement cannot be separated e.g. if several loggers write into the same file. As-is the individual statements may be scattered arbitrarily far in a sufficiently degenerate logger setup.

[Zsar]

This could have been another peek, for laconeia.

[martingrossmann]

Hm... the content of map() is to complex.

[Zsar]

Should work by constructing new XrayTestExecutionImport.TestRun(issue.getKey() inside updateTestInfoForImport, thereby eliminating its first parameter. The method is private.

[...]

	// Update test by mapper, convert to import entity and add to sync queue
	currentTestIssues.stream()
			.peek(issue -> xrayMapper.updateTest(issue, methodContext))
			.map(issue -> this.updateTestInfoForImport(issue, methodContext))
			.peek(testRun -> updateTestRunForImport(testRun, methodContext))
			.forEach(testRunSyncQueue::add);

	if (testRunSyncQueue.size() >= xrayConfig.getSyncFrequencyTests()) {
		flushSyncQueue();
	}
}

/**
 * Update the Info object for creating or updating Xray tests.
 * If no Info object is defined, the Xray test will not be updated.
 */
private XrayTestExecutionImport.TestRun updateTestInfoForImport(XrayTestIssue issue, MethodContext methodContext) {
	if (this.getXrayMapper().shouldCreateNewTest(methodContext)) {
		XrayTestExecutionImport.TestRun.Info info = new XrayTestExecutionImport.TestRun.Info();
		info.setDescription(issue.getDescription());
		info.setSummary(issue.getSummary());
		info.setLabels(issue.getLabels());
		info.setDefinition(issue.getSummary());
		info.setType(TestType.AutomatedGeneric);
		info.setProjectKey(issue.getProject().getKey());
		
		// The test's test type needs to be {@link TestType.Manual} to support test steps.
		info.setType(TestType.Manual);

		List<TestStep> testerraTestSteps = methodContext.readTestSteps().collect(Collectors.toList());
		for (TestStep testerraTestStep : testerraTestSteps) {
			if (testerraTestStep.isInternalTestStep()) {
				continue;
			}
			// The test steps definitions
			final XrayTestExecutionImport.TestStep importTestStep = new XrayTestExecutionImport.TestStep();
			importTestStep.setAction(testerraTestStep.getName());
			// We always expect the step to pass
			importTestStep.setResult(XrayTestExecutionImport.TestRun.Status.PASS.toString());
			info.addStep(importTestStep);
		}

		final var testRun = new XrayTestExecutionImport.TestRun(issue.getKey());
		testRun.setTestInfo(info);
		return testRun;
	}
}

... FWIW: This is not really function-al code to begin with. It might instead be better to not-Stream this and for-each it instead, to avoid the red herring:

// Update test by mapper, convert to import entity and add to sync queue
for (final var issue : currentTestIssues) {
	xrayMapper.updateTest(issue, methodContext);
	final var run = new XrayTestExecutionImport.TestRun(issue.getKey());
	this.updateTestInfoForImport(run, issue, methodContext);
	this.updateTestRunForImport(run, methodContext);
	testRunSyncQueue.add(run);
}

(without changes in the update[...] functions)

... But feel free to reject any of these ideas - it's not particularly important and I can also live with this part as-is.

[martingrossmann]

Good point. I moved the creation of a new TestRun into the method.

[Zsar]

Simple line comment instead of a three-line block containing one line of content?

[Zsar]

Should work by constructing new XrayTestExecutionImport.TestRun(issue.getKey() inside updateTestInfoForImport, thereby eliminating its first parameter. The method is private.

[...]

	// Update test by mapper, convert to import entity and add to sync queue
	currentTestIssues.stream()
			.peek(issue -> xrayMapper.updateTest(issue, methodContext))
			.map(issue -> this.updateTestInfoForImport(issue, methodContext))
			.peek(testRun -> updateTestRunForImport(testRun, methodContext))
			.forEach(testRunSyncQueue::add);

	if (testRunSyncQueue.size() >= xrayConfig.getSyncFrequencyTests()) {
		flushSyncQueue();
	}
}

/**
 * Update the Info object for creating or updating Xray tests.
 * If no Info object is defined, the Xray test will not be updated.
 */
private XrayTestExecutionImport.TestRun updateTestInfoForImport(XrayTestIssue issue, MethodContext methodContext) {
	if (this.getXrayMapper().shouldCreateNewTest(methodContext)) {
		XrayTestExecutionImport.TestRun.Info info = new XrayTestExecutionImport.TestRun.Info();
		info.setDescription(issue.getDescription());
		info.setSummary(issue.getSummary());
		info.setLabels(issue.getLabels());
		info.setDefinition(issue.getSummary());
		info.setType(TestType.AutomatedGeneric);
		info.setProjectKey(issue.getProject().getKey());
		
		// The test's test type needs to be {@link TestType.Manual} to support test steps.
		info.setType(TestType.Manual);

		List<TestStep> testerraTestSteps = methodContext.readTestSteps().collect(Collectors.toList());
		for (TestStep testerraTestStep : testerraTestSteps) {
			if (testerraTestStep.isInternalTestStep()) {
				continue;
			}
			// The test steps definitions
			final XrayTestExecutionImport.TestStep importTestStep = new XrayTestExecutionImport.TestStep();
			importTestStep.setAction(testerraTestStep.getName());
			// We always expect the step to pass
			importTestStep.setResult(XrayTestExecutionImport.TestRun.Status.PASS.toString());
			info.addStep(importTestStep);
		}

		final var testRun = new XrayTestExecutionImport.TestRun(issue.getKey());
		testRun.setTestInfo(info);
		return testRun;
	}
}

... FWIW: This is not really function-al code to begin with. It might instead be better to not-Stream this and for-each it instead, to avoid the red herring:

// Update test by mapper, convert to import entity and add to sync queue
for (final var issue : currentTestIssues) {
	xrayMapper.updateTest(issue, methodContext);
	final var run = new XrayTestExecutionImport.TestRun(issue.getKey());
	this.updateTestInfoForImport(run, issue, methodContext);
	this.updateTestRunForImport(run, methodContext);
	testRunSyncQueue.add(run);
}

(without changes in the update[...] functions)

... But feel free to reject any of these ideas - it's not particularly important and I can also live with this part as-is.