net/http/cgi: eliminate use of Perl in tests #63828

bcmills · 2023-10-30T18:00:27Z

net/http/cgi currently uses a Perl script (testdata/test.cgi) for some of its tests:
https://cs.opensource.google/search?q=perl%20file:go%20file:net%2Fhttp%2Fcgi&sq=&ss=go%2Fgo

Unfortunately, that can cause spurious failures due to bugs in the Perl script, such as:

Those failures are easily masked, because the tests are skipped on builders that do not have Perl installed (as appears to be the case for our Windows builders). The use of Perl also makes those tests harder for Go maintainers to review and maintain, since many of us are not very familiar with the Perl language.

I don't see a compelling reason why the test needs to use Perl for this, given that it could just as easily go build a handler written in Go.

See previously:

The text was updated successfully, but these errors were encountered:

raghvenders · 2023-10-30T19:24:49Z

@bcmills - Working on this. Is this only for test.cgi? I have seen at the high level it looks like returning html and handling query params. May be some more while implementing.

At high level all the tests should pass with go cgi handler?

bcmills · 2023-10-30T20:22:48Z

@raghvenders, I would expect that the test handler can be rewritten using cgi.Request to parse the input, and write the output in roughly the same manner as the existing Perl script.

To avoid the need to explicitly go build a CGI binary, the test binary itself can also function as the CGI handler, using an environment variable to tell the child process's TestMain to handle the CGI request instead of running the test. (For an example of that approach, see func TestMain in src/cmd/vet/vet_test.go.)

The calls to check(t) should be replaced with calls to testenv.MustHaveExec(t), and with that approach all of the tests should either pass (on most platforms) or skip (on js, wasip1, and perhaps ios, which do not support executing a subprocess).

raghvenders · 2023-11-01T04:34:21Z

@bcmills - Shall the cgi handler be like another child test process ?

h := &Handler{
		Path: os.Args[0], // cgi.test.exe
		Root: "/test.go",
		Args: []string{"-test.run=^GoCGIChildProcess$"},
	}

Let GoCGIChildProcess- handle and do output what test.cgi does currently.

the above handler will be available for all the test.cgi based tests like TestCGIBasicGet, TestDirWindows, etc.

Please do correct me on the understanding part of TestMain :

// TestMain executes the test binary as the go test command if
// GO_CGI_TEST is set, and runs the tests otherwise.
func TestMain(m *testing.M) {
	if os.Getenv("GO_CGI_TEST") != "" {
		main()
		os.Exit(0)
	}



	os.Setenv("GO_CGI_TEST", "1") // Set for subprocesses to inherit.
	os.Exit(m.Run())
}

we have to have above TestMain like this, so all the testcgi will inherit the above env - GO_CGI_TEST in their subprocess ?

Instead of test sub-process -test.run=^GoCGIChildProcess$, are you expecting GoCGIChildProcess as a GoCGIChildProcess.go file to have go run GoCGIChildProcess.go ?

The net/http/cgi package previously relied on a Perl script for some of its tests, which caused sporadic failures due to bugs in the Perl script. These failures were often masked because the tests were skipped on builders without Perl installed. However, this reliance on Perl made it more challenging for Go maintainers to review and maintain the code since not everyone was familiar with Perl. To address this issue and improve maintainability, I have removed the dependency on Perl in the tests. Instead, the tests now build a handler written in Go, ensuring consistent behavior across different environments. This change aligns with previous efforts to minimize or eliminate the use of Perl in various parts of the Go codebase golang#20032,golang#25586,golang#25669,golang#27779. By eliminating the Perl script from net/http/cgi tests, we can enhance the reliability and ease of maintenance for these tests going forward. Fixes golang#63800 Fixes golang#63828

aimuz · 2023-11-01T10:05:36Z

@bcmills Do you have time to review the code please, I've submitted a patch

#63869

gopherbot · 2023-11-01T10:09:47Z

Change https://go.dev/cl/538861 mentions this issue: net/http/cgi: eliminate use of Perl in tests

The net/http/cgi package previously relied on a Perl script for some of its tests, which caused sporadic failures due to bugs in the Perl script. These failures were often masked because the tests were skipped on builders without Perl installed. However, this reliance on Perl made it more challenging for Go maintainers to review and maintain the code since not everyone was familiar with Perl. To address this issue and improve maintainability, I have removed the dependency on Perl in the tests. Instead, the tests now build a handler written in Go, ensuring consistent behavior across different environments. This change aligns with previous efforts to minimize or eliminate the use of Perl in various parts of the Go codebase golang#20032,golang#25586,golang#25669,golang#27779. By eliminating the Perl script from net/http/cgi tests, we can enhance the reliability and ease of maintenance for these tests going forward. Fixes golang#63800 Fixes golang#63828 Change-Id: I8d554af93d4070036cf0cc3aaa9c9b256affbd17 GitHub-Last-Rev: b652d18 GitHub-Pull-Request: golang#63869

The net/http/cgi package previously relied on a Perl script for some of its tests, which caused sporadic failures due to bugs in the Perl script. These failures were often masked because the tests were skipped on builders without Perl installed. However, this reliance on Perl made it more challenging for Go maintainers to review and maintain the code since not everyone was familiar with Perl. To address this issue and improve maintainability, I have removed the dependency on Perl in the tests. Instead, the tests now build a handler written in Go, ensuring consistent behavior across different environments. This change aligns with previous efforts to minimize or eliminate the use of Perl in various parts of the Go codebase golang#20032,golang#25586,golang#25669,golang#27779. By eliminating the Perl script from net/http/cgi tests, we can enhance the reliability and ease of maintenance for these tests going forward. Fixes golang#63800 Fixes golang#63828

The net/http/cgi package previously relied on a Perl script for some of its tests, which caused sporadic failures due to bugs in the Perl script. These failures were often masked because the tests were skipped on builders without Perl installed. However, this reliance on Perl made it more challenging for Go maintainers to review and maintain the code since not everyone was familiar with Perl. To address this issue and improve maintainability, I have removed the dependency on Perl in the tests. Instead, the tests now build a handler written in Go, ensuring consistent behavior across different environments. This change aligns with previous efforts to minimize or eliminate the use of Perl in various parts of the Go codebase By eliminating the Perl script from net/http/cgi tests, we can enhance the reliability and ease of maintenance for these tests going forward. Fixes golang#63800 Fixes golang#63828

Previously, a Perl script was used to test the net/http/cgi package. This sometimes led to hidden failures as these tests were not run on builders without Perl. Also, this approach posed maintenance difficulties for those unfamiliar with Perl. We have now replaced Perl-based tests with a Go handler to simplify maintenance and ensure consistent testing environments. It's part of our ongoing effort to reduce reliance on Perl throughout the Go codebase (see golang#20032,golang#25586,golang#25669,golang#27779), thus improving reliability and ease of maintenance. Fixes golang#63800 Fixes golang#63828 Change-Id: I8d554af93d4070036cf0cc3aaa9c9b256affbd17 GitHub-Last-Rev: c44cd69 GitHub-Pull-Request: golang#63869

bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Oct 30, 2023

bcmills added this to the Backlog milestone Oct 30, 2023

bcmills mentioned this issue Oct 30, 2023

net/http/cgi: TestDirWindows fails on Windows when perl is installed #63800

Closed

bcmills changed the title ~~net/http/cgi: eliminate use of Perl is tests~~ net/http/cgi: eliminate use of Perl in tests Oct 30, 2023

aimuz mentioned this issue Nov 1, 2023

net/http/cgi: eliminate use of Perl in tests #63869

Closed

gopherbot closed this as completed in ff19f8e Nov 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net/http/cgi: eliminate use of Perl in tests #63828

net/http/cgi: eliminate use of Perl in tests #63828

bcmills commented Oct 30, 2023

raghvenders commented Oct 30, 2023

bcmills commented Oct 30, 2023

raghvenders commented Nov 1, 2023

aimuz commented Nov 1, 2023

gopherbot commented Nov 1, 2023

net/http/cgi: eliminate use of Perl in tests #63828

net/http/cgi: eliminate use of Perl in tests #63828

Comments

bcmills commented Oct 30, 2023

raghvenders commented Oct 30, 2023

bcmills commented Oct 30, 2023

raghvenders commented Nov 1, 2023

aimuz commented Nov 1, 2023

gopherbot commented Nov 1, 2023