Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/refactor/rename: TestMoves is failing on Windows #16384

Closed
bradfitz opened this issue Jul 15, 2016 · 4 comments
Closed

x/tools/refactor/rename: TestMoves is failing on Windows #16384

bradfitz opened this issue Jul 15, 2016 · 4 comments
Labels
FrozenDueToAge help wanted OS-Windows Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@bradfitz
Copy link
Contributor

--- FAIL: TestMoves (0.01s)
    mvpkg_test.go:403: -from "b" -to "x/a": unexpected warnings:
        \go\src\conflict\0.go:4:8: renaming this imported package name "b" to "a"
        \go\src\conflict\0.go:3:8:  conflicts with imported package name in same block
        \go\src\conflict\0.go:3:8: skipping update of this file
        want:
        /go/src/conflict/0.go:4:8: renaming this imported package name "b" to "a"
        /go/src/conflict/0.go:3:8:  conflicts with imported package name in same block
        /go/src/conflict/0.go:3:8: skipping update of this file
    mvpkg_test.go:418: -from "b" -to "x/a": rewritten file /go/src/ok/0.go does not match expectation; got <<<package ok

        import a "x\\a"

        var _ a.B
        >>>
        want <<<package ok

        import "x/a"

        var _ a.B
        >>>
    mvpkg_test.go:418: -from "b" -to "x/a": rewritten file /go/src/conflict/0.go does not match expectation; got <<<package conflict

        import "a"
        import a "x\\a"

        var _ a.A
        var _ b.B
        >>>
        want <<<package conflict

        import "a"
        import "x/a"

        var _ a.A
        var _ b.B
        >>>
    mvpkg_test.go:395: -from "x/foo" -to "y/foo": unexpected error: invalid move destination: y/foo; package or subpackage x/foo already exists
Renamed 1 occurrence in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 1 occurrence in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 1 occurrence in 1 file in 1 package.
Renamed 1 occurrence in 1 file in 1 package.
Renamed 3 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 2 files in 2 packages.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 4 occurrences in 1 file in 1 package.
Renamed 4 occurrences in 1 file in 1 package.
Renamed 4 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 3 occurrences in 2 files in 2 packages.
Renamed 3 occurrences in 2 files in 2 packages.
Renamed 3 occurrences in 2 files in 2 packages.
Renamed 5 occurrences in 1 file in 1 package.
Renamed 5 occurrences in 1 file in 1 package.
Renamed 3 occurrences in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 1 occurrence in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
Renamed 1 occurrence in 1 file in 1 package.
Renamed 2 occurrences in 1 file in 1 package.
FAIL
FAIL    golang.org/x/tools/refactor/rename  0.213s

/cc @quentinmit (feel free to file bugs like this for all subrepo failures if none exist)

@bradfitz bradfitz added the Testing An issue that has been verified to require only test changes, not just a test failure. label Jul 15, 2016
@bradfitz bradfitz added this to the Unreleased milestone Jul 15, 2016
@gopherbot
Copy link

CL https://golang.org/cl/24941 mentions this issue.

gopherbot pushed a commit to golang/tools that referenced this issue Jul 15, 2016
I felt the burn of my laptop on my legs, spinning away while processing
goimports, and felt that it was time to make goimports great again.

Over the past few years goimports fell into a slow state of disrepair
with too many feature additions and no attention to the performance
death by a thousand cuts. This was particularly terrible on OS X with
its lackluster filesystem buffering.

This CL makes goimports stronger, together with various optimizations
and more visibility into what goimports is doing.

* adds more internal documentation

* avoids scanning $GOPATH for answers when running goimports on a file
  under $GOROOT (for Go core hackers)

* don't read all $GOROOT & $GOPATH directories' Go code looking for
  their package names until much later. Require the package name of
  missing imports to be present in the last two directory path
  components.  Then only try importing them in order from best to
  worst (shortest to longest, as before), so we can stop early.

* when adding imports, add names to imports when the imported package name
  doesn't match the baes of its import path. For example:
        import foo "example.net/foo/v1"

* don't read all *.go files in a package directory once the first file
  in a directory has revealed itself to be a package we're not looking
  for. For example, if we're looking for the right "client" for "client.Foo",
  we used to consider a directory "bar/client" as a candidate and read
  all 50 of its *.go files instead of stopping after its first *.go
  file had a "package main" line.

* add some fast paths to remove allocations

* add some fast paths to remove disk I/O when looking up the base
  package name of a standard library import (of existing imports in a
  file, which are very common)

* adds a special case for import "C", to avoid some disk I/O.

* add a -verbose flag to goimports for debugging

On my Mac laptop with a huge $GOPATH, with a test file like:

	package foo
	import (
	       "fmt"
	       "net/http"
	)

	/*

	*/
	import "C"

	var _ = cloudbilling.New
	var _ = http.NewRequest
	var _ = client.New

... this took like 10 seconds before, and now 1.3 seconds. (Still
slow; disk-based caching can come later)

Updates golang/go#16367 (goimports is slow)
Updates golang/go#16384 (refactor TestRename is broken on Windows)

Change-Id: I97e85d3016afc9f2ad5501f97babad30c7989183
Reviewed-on: https://go-review.googlesource.com/24941
Reviewed-by: Andrew Gerrand <adg@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@adonovan
Copy link
Member

Looks like two separate issues, both related to forward vs. backslashes as directory separators:

  • the import declaration uses backslashes (a bug in the package)
  • the actual warnings in the test use backslashes (a bug in the test)

@gopherbot
Copy link

CL https://golang.org/cl/24969 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/32392 mentions this issue.

@golang golang locked and limited conversation to collaborators Oct 31, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted OS-Windows Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

4 participants