Remove newlineReporter custom report

[chendave]

The newlineReporter intends to print a new line after the test to prevent the something print to the stdout and mess up the test result while cause the tool like go-junit-report fail to parse the result.

But this is no longer needed based on following evidence.

Tested with this patch,

diff --git a/staging/src/k8s.io/kubectl/pkg/apps/apps_suite_test.go b/staging/src/k8s.io/kubectl/pkg/apps/apps_suite_test.go
index 86296f75f6c..850c86e8447 100644
--- a/staging/src/k8s.io/kubectl/pkg/apps/apps_suite_test.go
+++ b/staging/src/k8s.io/kubectl/pkg/apps/apps_suite_test.go
@@ -29,6 +29,7 @@ import (
 func TestApps(t *testing.T) {
        RegisterFailHandler(Fail)
        RunSpecsWithDefaultAndCustomReporters(t, "Apps Suite", []Reporter{newlineReporter{}})
+       fmt.Print("hello world stdout")
 }

 // Print a newline after the default newlineReporter due to issue
@@ -46,4 +47,4 @@ func (newlineReporter) SpecWillRun(specSummary *SpecSummary) {}
 func (newlineReporter) SpecDidComplete(specSummary *SpecSummary) {}

 // SpecSuiteDidEnd Prints a newline between "35 Passed | 0 Failed | 0 Pending | 0 Skipped" and "--- PASS:"
-func (newlineReporter) SpecSuiteDidEnd(summary *SuiteSummary) { fmt.Printf("\n") }
+func (newlineReporter) SpecSuiteDidEnd(summary *SuiteSummary) { fmt.Printf("hello world \n") }

# go test -v
=== RUN   TestApps
Running Suite: Apps Suite
=========================
Random Seed: 1654060709
Will run 9 of 9 specs

•••••••••hello world

Ran 9 of 9 Specs in 0.000 seconds
SUCCESS! -- 9 Passed | 0 Failed | 0 Pending | 0 Skipped
hello world stdout--- PASS: TestApps (0.00s)
PASS
ok      k8s.io/kubectl/pkg/apps 0.005s

• The issue that was first introduced in go-junit-report has already fixed in the version
  referenced in go.mod.

It was fixed here,
jstemmer/go-junit-report#64

On the mater is here,
https://github.com/jstemmer/go-junit-report/blob/9ad16898a8044f83800984add0907d4e1c070ecb/parser/gotest/gotest.go#L22
It is go-junit-report v0.9.1 referenced in the go.mod which has already included the fix.

And I doubt that there is any direct dependency on go-junit-report in Kubernetes.

• The newlineReporter report doesn't fix anything for Ginkgo v1 or V2, it just prints a
  new line before the test summarization.

# ginkgo.v1
Running Suite: Apps Suite
=========================
Random Seed: 1654062151
Will run 9 of 9 specs

•••••••••hello world

Ran 9 of 9 Specs in 0.000 seconds
SUCCESS! -- 9 Passed | 0 Failed | 0 Pending | 0 Skipped
hello world stdout
PASS

Ginkgo ran 1 suite in 718.021634ms
Test Suite Passed

The newline is print right above the line of Ran 9 of 9 Specs in 0.000 seconds

ref:
[1] jstemmer/go-junit-report#31
[2] jstemmer/go-junit-report#64
[3] jstemmer/go-junit-report#63 (the output from the Ginkgo is no longer applicable for Ginkgo v1.14.0 or Ginkgo V2.1.4

/cc @pohly
/cc @aojea
/cc @onsi
/cc @liggitt
/cc @BenTheElder

A following up for this: #109111

What type of PR is this?

Add one of the following kinds:
/kind cleanup

What this PR does / why we need it:

Which issue(s) this PR fixes:

Partial fixes ##109744

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

[k8s-ci-robot]

@chendave: GitHub didn't allow me to request PR reviews from the following users: onsi.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

The newlineReporter intends to print a new line after the test to prevent the something print to the stdout and mess up the test result while cause the tool like go-junit-report fail to parse the result.

But this is no longer needed based on following evidence.

• The newline already added before --- PASS on go 1.18 regardless there is anything printed
  to stdout.

Tested with this patch,

diff --git a/staging/src/k8s.io/kubectl/pkg/apps/apps_suite_test.go b/staging/src/k8s.io/kubectl/pkg/apps/apps_suite_test.go
index 86296f75f6c..850c86e8447 100644
--- a/staging/src/k8s.io/kubectl/pkg/apps/apps_suite_test.go
+++ b/staging/src/k8s.io/kubectl/pkg/apps/apps_suite_test.go
@@ -29,6 +29,7 @@ import (
func TestApps(t *testing.T) {
       RegisterFailHandler(Fail)
       RunSpecsWithDefaultAndCustomReporters(t, "Apps Suite", []Reporter{newlineReporter{}})
+       fmt.Println("hello world stdout")
}

// Print a newline after the default newlineReporter due to issue
@@ -46,4 +47,4 @@ func (newlineReporter) SpecWillRun(specSummary *SpecSummary) {}
func (newlineReporter) SpecDidComplete(specSummary *SpecSummary) {}

// SpecSuiteDidEnd Prints a newline between "35 Passed | 0 Failed | 0 Pending | 0 Skipped" and "--- PASS:"
-func (newlineReporter) SpecSuiteDidEnd(summary *SuiteSummary) { fmt.Printf("\n") }
+func (newlineReporter) SpecSuiteDidEnd(summary *SuiteSummary) { fmt.Printf("hello world \n") }

# go test -v
=== RUN   TestApps
Running Suite: Apps Suite
=========================
Random Seed: 1654060709
Will run 9 of 9 specs

•••••••••hello world

Ran 9 of 9 Specs in 0.000 seconds
SUCCESS! -- 9 Passed | 0 Failed | 0 Pending | 0 Skipped
hello world stdout
--- PASS: TestApps (0.00s)
PASS
ok      k8s.io/kubectl/pkg/apps 0.005s

• The issue that was first introduced in go-junit-report has already fixed in the version
  referenced in go.mod.

It was fixed here,
jstemmer/go-junit-report#64

On the mater is here,
https://github.com/jstemmer/go-junit-report/blob/9ad16898a8044f83800984add0907d4e1c070ecb/parser/gotest/gotest.go#L22
It is go-junit-report v0.9.1 referenced in the go.mod which has already included the fix.

And I doubt that there is any direct dependency on go-junit-report in Kubernetes.

• The newlineReporter report doesn't fix anything for Ginkgo v1 or V2, it just prints a
  new line before the test summarization.

# ginkgo.v1
Running Suite: Apps Suite
=========================
Random Seed: 1654062151
Will run 9 of 9 specs

•••••••••hello world

Ran 9 of 9 Specs in 0.000 seconds
SUCCESS! -- 9 Passed | 0 Failed | 0 Pending | 0 Skipped
hello world stdout
PASS

Ginkgo ran 1 suite in 718.021634ms
Test Suite Passed

The newline is print right above the line of Ran 9 of 9 Specs in 0.000 seconds

ref:
[1] jstemmer/go-junit-report#31
[2] jstemmer/go-junit-report#64
[3] jstemmer/go-junit-report#63 (the output from the Ginkgo is no longer applicable for Ginkgo v1.14.0 or Ginkgo V2.1.4

/cc @pohly
/cc @aojea
/cc @onsi
/cc @liggitt
/cc @BenTheElder

A following up for this: #109111

What type of PR is this?

Add one of the following kinds:
/kind cleanup

What this PR does / why we need it:

Which issue(s) this PR fixes:

Partial fixes ##109744

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

@chendave: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[pohly]

But this is no longer needed based on following evidence.
...
fmt.Println("hello world stdout")

What happens when the test writes something to stdout without a newline at the end? I.e. when using fmt.Print("hello world stdout")? Is that handled by the updated go-junit-report?

However, I lack some background here. It is not clear to me what the original problem was and why go-junit-report is used in combination with a Ginkgo test suite.

[chendave]

What happens when the test writes something to stdout without a newline at the end? I.e. when using fmt.Print("hello world stdout")? Is that handled by the updated go-junit-report?

Thanks, I should use fmt.Print instead of fmt.Println in the example, updated the description, so the output will looks as this,
hello world stdout--- PASS: TestApps (0.00s)
this was fixed by go-junit-report, pls see the link I provided in the pr description.

However, I lack some background here.

Me too, but from what was observed by the Ginkgo or go test, this newlineReporter report doesn't look like fix anything and the original issue was fixed in go-junit-report although I am not sure how this project tied with Kubernetes.

I saw that @eddiezane is the co-chair of the sig-cli, maybe he know more than us, or point us to the right person who can step out for a review?

[pohly]

The description now makes more sense to me because it actually reproduces the original problem.

/lgtm

[pohly]

/assign @brianpursle

For approval.

[k8s-ci-robot]

@pohly: GitHub didn't allow me to assign the following users: brianpursle.

Note that only kubernetes members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @brianpursle

For approval.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[dims]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chendave, dims

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• staging/src/k8s.io/kubectl/OWNERS [dims]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-triage-robot]

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

• The PR does have any do-not-merge/* labels
• The PR does not have the needs-ok-to-test label
• The PR is mergeable (does not have a needs-rebase label)
• The PR is approved (has cncf-cla: yes, lgtm, approved labels)
• The PR is failing tests required for merge

You can:

• Review the full test history for this PR
• Prevent this bot from retesting with /lgtm cancel or /hold
• Help make our tests less flaky by following our Flaky Tests Guide

/retest