configure cmd for junit #6574

stevekuznetsov · 2016-01-07T18:57:55Z

This pull does the following:

sets up a jUnit output text file artifact
gets tests using os::cmd to output to it
sets up test suite and test case bookends
inserts test suite markings into test/cmd/*.sh
formats tests to better break things up

@deads2k @liggitt @soltysh PTAL - I suggest to look at the first commit only to see changes, or look at the entire changes with whitespace disabled.

soltysh · 2016-01-11T14:18:52Z

LGTM

soltysh · 2016-01-11T14:19:12Z

Although I'm still thinking about those indentations... ;)

deads2k · 2016-01-11T14:29:53Z

Although I'm still thinking about those indentations... ;)

@stevekuznetsov I think I'm against indenting like that. I don't think it enhances the readability of the script.

liggitt · 2016-01-11T14:32:00Z

@stevekuznetsov I think I'm against indenting like that. I don't think it enhances the readability of the script.

Agree. I wouldn't expect more than one "package" per test-cmd script, honestly.

deads2k · 2016-01-11T14:33:12Z

Agree. I wouldn't expect more than one package per test-cmd script, honestly.

Eh, I'm ok with multiple packages. We sort of did that with "X set of tests: ok".

deads2k · 2016-01-11T14:35:09Z

hack/cmd_util.sh

@@ -182,16 +184,18 @@ function os::cmd::internal::expect_exit_code_run_grep() {
 	done

 	if (( cmd_succeeded && test_succeeded )); then
-		os::text::print_green "SUCCESS after ${time_elapsed}s: ${name}"
+		os::text::print_green "SUCCESS after ${time_elapsed}s: ${name}" | tee "{JUNIT_REPORT_OUTPUT}"


I would have expected tee "${JUNIT_REPORT_OUTPUT}". Is the dollar sign not needed for some reason?

Someone is going to call this method without JUNIT_RESPORT_OUTPUT set. Make this command work as expected for them and add a test to the tests for this file to prove that it works.

Make this command work as expected

Without that variable set right now:

no file is written

no error is raised

nothing fails to run

What else is expected?

deads2k · 2016-01-11T14:40:21Z

A few comments. I largely like the change.

stevekuznetsov · 2016-01-14T14:03:25Z

hack/test-cmd.sh

@@ -55,6 +56,11 @@ tests=( $(find_tests ${1:-.*}) )

 # Setup environment

+if [[ -z "${JUNIT_REPORT_OUTPUT+x}" ]]; then
+  # if no jUnit report output file is set, throw away our jUnit info
+  JUNIT_REPORT_OUTPUT="/dev/null"


@deads2k the jUnit functions need $JUNIT_REPORT_OUTPUT to be set. I don't think it's reasonable to stick a check in every single function, so instead an if case like this in any script using jUnit is more appropriate.

stevekuznetsov · 2016-01-14T14:07:05Z

@deads2k ready for another look

deads2k · 2016-01-18T14:38:43Z

Without that variable set right now:

no file is written
no error is raised
nothing fails to run
What else is expected?

You're setting it in test-cmd, but every single place that calls these methods will need to do that same detection and set to /dev/null. That's unreasonable.

deads2k · 2016-01-18T14:38:46Z

[test]

deads2k · 2016-01-18T14:40:00Z

hack/cmd_util.sh

 	fi
 }

 # os::cmd::internal::mark_attempt marks the end of an attempt in the stdout and stderr log files
 # this is used to make the try_until_* output more concise
 function os::cmd::internal::mark_attempt() {
-	echo -e '\x1e' >> "${os_cmd_internal_tmpout}" | tee "${os_cmd_internal_tmperr}"
+	echo -e '\x1e' >> "${os_cmd_internal_tmpout}"


Why did this change?

soltysh · 2016-03-24T16:51:13Z

hack/test-cmd.sh

+os::util::sed '5,10d' ${BASETMPDIR}/node-config-broken.yaml
+os::cmd::expect_failure_and_text "openshift ex validate node-config ${BASETMPDIR}/node-config-broken.yaml" 'ERROR'
+echo "validation: ok"
+os::test::junit::declare_suite_end


Since in some places we do, others we don't echo "<group test>: ok", how about adding those output as part of os::test::junit::declare_suite_start and os::test::junit::declare_suite_end. These know exactly what test suite your in and can output that information. One less line of typing and you have similar output both in the console and in the report, wdyt?

It's an interesting thought. I think before os::cmd these types of declarations had value in that it was difficult to tell if forward progress was being made or not. What do we gain by keeping them at all now? Also, I decided to not add suite-specific information to os::test::junit:: declare_suite_end as without a stack that gets hairy on reconciliation.

This can be applied as a followup, of course :)

soltysh · 2016-03-24T17:03:59Z

Still LGTM.

stevekuznetsov · 2016-04-06T12:40:42Z

@deads2k @soltysh rebased, up-to-date again

deads2k · 2016-04-06T12:42:53Z

post-1.2

@soltysh has the merge.

stevekuznetsov · 2016-04-06T14:44:39Z

Tests succeeded but failed on the last step - publishing jUnit XML report. I think @smarterclayton turned that on. Otherwise, green.

smarterclayton · 2016-04-06T14:53:57Z

Going to do this post 1.2 once I cut the branch.

stevekuznetsov · 2016-04-08T17:01:54Z

re[test] for good luck

stevekuznetsov · 2016-04-08T19:52:56Z

Flakes:
https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/14321/: #7429
https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/14320: #8433

re[test]

smarterclayton · 2016-04-11T22:45:52Z

[test]

On Fri, Apr 8, 2016 at 5:25 PM, OpenShift Bot [email protected]
wrote:

continuous-integration/openshift-jenkins/test FAILURE (
https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/2862/)

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#6574 (comment)

stevekuznetsov · 2016-04-13T14:05:03Z

conformance flakes on OOM issue: kubernetes/kubernetes#23894
integration flake on TestNewAppRunBuilds:

--- FAIL: TestNewAppRunBuilds-2 (3.15s)
    newapp_test.go:1198: successful build from dockerfile with --to: unexpected error: no match for "openshift/origin-base"

re[test]

openshift-bot · 2016-04-13T14:05:26Z

Evaluated for origin test up to 47ace9d

openshift-bot · 2016-04-13T15:01:37Z

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/2968/)

stevekuznetsov · 2016-04-15T14:19:28Z

@soltysh ping

soltysh · 2016-04-15T14:27:37Z

@stevekuznetsov still waiting for 1.2 cut

smarterclayton · 2016-04-15T15:38:33Z

This is approved for master before 1.2 because it is test infrastructure.

soltysh · 2016-04-18T08:55:38Z

Upon approval:
[merge]

openshift-bot · 2016-04-18T09:00:15Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5609/) (Image: devenv-rhel7_3988)

openshift-bot · 2016-04-18T09:00:16Z

Evaluated for origin merge up to 47ace9d

stevekuznetsov changed the title ~~Skuznets/configure cmd for junit~~ configure cmd for junit Jan 7, 2016

openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 7, 2016

deads2k reviewed Jan 11, 2016
View reviewed changes

stevekuznetsov force-pushed the skuznets/configure-cmd-for-junit branch from 1294b48 to 758b23e Compare January 14, 2016 14:00

stevekuznetsov reviewed Jan 14, 2016
View reviewed changes

stevekuznetsov force-pushed the skuznets/configure-cmd-for-junit branch from 758b23e to 2f30d24 Compare January 14, 2016 14:05

stevekuznetsov force-pushed the skuznets/configure-cmd-for-junit branch from 2f30d24 to f3af403 Compare January 14, 2016 14:23

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 14, 2016

deads2k reviewed Jan 18, 2016
View reviewed changes

stevekuznetsov force-pushed the skuznets/configure-cmd-for-junit branch 4 times, most recently from 74e2b18 to a00d89e Compare January 18, 2016 19:42

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 20, 2016

soltysh reviewed Mar 24, 2016
View reviewed changes

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 3, 2016

stevekuznetsov added 4 commits April 6, 2016 08:38

configure test-cmd to emit parsable output for jUnit

eb76764

refactored nested suites builder

49a99fc

added support for os::cmd parsing to junitreport

18a479e

added jUnit XML publishing step to exit trap

47ace9d

stevekuznetsov force-pushed the skuznets/configure-cmd-for-junit branch from 28c7c59 to 47ace9d Compare April 6, 2016 12:38

deads2k added this to the 1.2.x milestone Apr 6, 2016

deads2k assigned soltysh Apr 6, 2016

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 6, 2016

openshift-bot merged commit fc404cc into openshift:master Apr 18, 2016

stevekuznetsov deleted the skuznets/configure-cmd-for-junit branch June 15, 2016 18:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

configure cmd for junit #6574

configure cmd for junit #6574

stevekuznetsov commented Jan 7, 2016

soltysh commented Jan 11, 2016

soltysh commented Jan 11, 2016

deads2k commented Jan 11, 2016

liggitt commented Jan 11, 2016

deads2k commented Jan 11, 2016

deads2k Jan 11, 2016

deads2k Jan 11, 2016

stevekuznetsov Jan 14, 2016

deads2k commented Jan 11, 2016

stevekuznetsov Jan 14, 2016

stevekuznetsov commented Jan 14, 2016

deads2k commented Jan 18, 2016

deads2k commented Jan 18, 2016

deads2k Jan 18, 2016

soltysh Mar 24, 2016

stevekuznetsov Mar 24, 2016

soltysh Mar 24, 2016

soltysh commented Mar 24, 2016

stevekuznetsov commented Apr 6, 2016

deads2k commented Apr 6, 2016

stevekuznetsov commented Apr 6, 2016

smarterclayton commented Apr 6, 2016

stevekuznetsov commented Apr 8, 2016

stevekuznetsov commented Apr 8, 2016

smarterclayton commented Apr 11, 2016

stevekuznetsov commented Apr 13, 2016

openshift-bot commented Apr 13, 2016

openshift-bot commented Apr 13, 2016

stevekuznetsov commented Apr 15, 2016

soltysh commented Apr 15, 2016

smarterclayton commented Apr 15, 2016

soltysh commented Apr 18, 2016

openshift-bot commented Apr 18, 2016

openshift-bot commented Apr 18, 2016

configure cmd for junit #6574

configure cmd for junit #6574

Conversation

stevekuznetsov commented Jan 7, 2016

soltysh commented Jan 11, 2016

soltysh commented Jan 11, 2016

deads2k commented Jan 11, 2016

liggitt commented Jan 11, 2016

deads2k commented Jan 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Jan 11, 2016

Choose a reason for hiding this comment

stevekuznetsov commented Jan 14, 2016

deads2k commented Jan 18, 2016

deads2k commented Jan 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soltysh commented Mar 24, 2016

stevekuznetsov commented Apr 6, 2016

deads2k commented Apr 6, 2016

stevekuznetsov commented Apr 6, 2016

smarterclayton commented Apr 6, 2016

stevekuznetsov commented Apr 8, 2016

stevekuznetsov commented Apr 8, 2016

smarterclayton commented Apr 11, 2016

stevekuznetsov commented Apr 13, 2016

openshift-bot commented Apr 13, 2016

openshift-bot commented Apr 13, 2016

stevekuznetsov commented Apr 15, 2016

soltysh commented Apr 15, 2016

smarterclayton commented Apr 15, 2016

soltysh commented Apr 18, 2016

openshift-bot commented Apr 18, 2016

openshift-bot commented Apr 18, 2016