Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Test tip on travis #236

Merged
merged 7 commits into from
Feb 14, 2018
Merged

Test tip on travis #236

merged 7 commits into from
Feb 14, 2018

Conversation

keegancsmith
Copy link
Member

We need to sort out our tests, they currently fail.

@keegancsmith
Test tip on travis
ca407fb
@keegancsmith
Test definition which hasn't change in goroot for a long time
aeb751f 
fmt.Println seems to have changed location between go1.9 and tip.
@keegancsmith
Copy link
Member Author

@Contextualist do you mind taking a look? The only thing failing now on go1.10 is completiion:

--- FAIL: TestServer (11.77s)
    --- FAIL: TestServer/go_vendored_dep (0.61s)
        langserver_test.go:1265: $ go install -v ...
                test/pkg/vendor/github.com/v/vendored
                test/pkg
        --- FAIL: TestServer/go_vendored_dep/completion-a.go:1:34 (0.00s)
                langserver_test.go:1422: got "", want "1:20-1:34 github.com/v/vendored module "
        --- FAIL: TestServer/go_vendored_dep/completion-a.go:1:61 (0.00s)
                langserver_test.go:1422: got "", want "1:61-1:61 V function func()"

lloiser
lloiser reviewed Feb 13, 2018
View reviewed changes
langserver/langserver_test.go Outdated
@@ -449,7 +449,7 @@ package main; import "test/pkg"; func B() { p.A(); B() }`,
// "a.go:1:53": "type int int",
},
overrideGodefDefinition: map[string]string{
"a.go:1:40": "/goroot/src/fmt/print.go:256:6-256:13", // hitting the real GOROOT
"a.go:1:71": "/goroot/src/fmt/print.go:38:6-38:11", // hitting the real GOROOT
Copy link
Contributor

Choose a reason for hiding this comment

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

Every small change in the src will fail this test case. Is there a better approach to this? Maybe just assert that a valid file and line number is returned?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah that sounds much better. It requires a bit of cleaning up in our tests, but that seems fine given this is relying on GOROOT of the user running the tests. Right now the location I have picked hasn't changed in 2 years.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to test path instead for this specific test.

@keegancsmith
Revert "Test definition which hasn't change in goroot for a long time"
4935ba8 
This reverts commit aeb751f.
@keegancsmith
Only test if godef to GOROOT matches path
3247dad
@keegancsmith
Install all instead of ...
db373e4 
go1.10 introduces the cache directory. So now objects will only appear in `pkg` if
they are explicitly installed. Previous versions of go would create entries for
the transitive dependencies. So for our vendor tests running `go install -i ...`
would not create a corresponding file in `pkg` for the vendored packages since
`...` does not list vendor. Using `all` instead does include the vendored pkg,
so will include the vendored pkgs. This will likely have wider effects on go
tooling that rely on pkg. Already filed nsf/gocode#500
for gocode.
@Contextualist
Copy link
Contributor

@keegancsmith Ah! Thanks for fixing the vendor issue. I just found that .a files for vendor packages are missing, never had thought that it is the problem due to go 1.10 package caching.

@keegancsmith
Copy link
Member Author

@Contextualist yeah sorry I should of mentioned I decided to look into it.

@Contextualist
Copy link
Contributor

@keegancsmith That's totally fine. 😄 Appreciate your efficiency!

Nonetheless, we have to wait for gocode to fix the issue, otherwise go1.10 users have to either install vendor package manually or disable package caching.

@keegancsmith keegancsmith changed the title WIP Test tip on travis Test tip on travis Feb 14, 2018
@keegancsmith keegancsmith merged commit 20886c7 into master Feb 14, 2018
@keegancsmith keegancsmith deleted the test-tip branch February 14, 2018 10:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lloiser lloiser lloiser left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@keegancsmith @Contextualist @lloiser