Skip to content

windows: Fix various non-portable tests#40986

Open
darinpp wants to merge 1 commit intocockroachdb:masterfrom
darinpp:master
Open

windows: Fix various non-portable tests#40986
darinpp wants to merge 1 commit intocockroachdb:masterfrom
darinpp:master

Conversation

@darinpp
Copy link
Copy Markdown
Contributor

@darinpp darinpp commented Sep 23, 2019

Addresses non-portable tests that are failing on Windows.

part of #39079

Release note: None

@darinpp darinpp requested a review from bdarnell September 23, 2019 17:44
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@darinpp darinpp force-pushed the master branch 2 times, most recently from da1fee3 to b277acc Compare September 23, 2019 20:10
Addresses tests that are failing on Windows because they are written
in a non-portable manner.

part of cockroachdb#39079

Release note: None
Copy link
Copy Markdown
Contributor

@bdarnell bdarnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 5 of 5 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @darinpp)


pkg/base/addr_validation_test.go, line 98 at r1 (raw file):

			addrs{hostAddr + ":26257", hostname + ":12345", hostAddr + ":8080", hostname + ":8080", hostAddr + ":5432", hostname + ":5432"}},

		// Use a non-numeric port number.

Comment that we're using pop3 because it's old enough to be ubiquitous in port mapping files across all platforms.


pkg/base/store_spec_test.go, line 160 at r1 (raw file):

		if testCase.expected.Path != "" {
			// In case that the testCase has relative path
			testCase.expected.Path, _ = filepath.Abs(testCase.expected.Path)

We should always check errors instead of discarding them.


pkg/server/status_test.go, line 440 at r1 (raw file):

	t.Run("path separators", func(t *testing.T) {
		// pattern/with/separators on unix
		// pattern\with\separators on windows

I'm not sure if this is what we want. The code tested here is part of debug zip, which won't know to change path separators around based on the platform of the target node. It doesn't matter for now because there doesn't seem to be any current use of this function with a path separator, but it could easily happen. I think it would be better to define the GetFiles interface in terms of URL/posix-style paths and make the GetFiles implementation responsible for translating those paths to those of the local system.

@tbg tbg added the X-noremind Bots won't notify about PRs with X-noremind label May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

X-noremind Bots won't notify about PRs with X-noremind

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants