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.

Sign up for GitHub

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

Jump to bottom

testing: Long test on windows is broken #832

Open
hyangah opened this issue Oct 23, 2020 · 10 comments
Open

testing: Long test on windows is broken #832

hyangah opened this issue Oct 23, 2020 · 10 comments
Milestone
vscode-go/backlog

Comments

@hyangah
Copy link
Contributor

hyangah commented Oct 23, 2020

See https://github.com/golang/vscode-go/runs/1289011464?check_suite_focus=true
@hyangah hyangah added this to the On Deck milestone Oct 23, 2020
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/266418 mentions this issue: test/gopls: change test environment setup to use single file edit

gopherbot pushed a commit that referenced this issue Nov 10, 2020
@hyangah
test/gopls: change test environment setup to use single file edit
e81c30f 
When we wrote the first gopls integration test, gopls required
vscode to open the workspace folder while vscode code does not
support dynamic workspace folder registration in test mode. As a
result, we ended up having a fixed workspace folder as a test fixture,
starting the test code instance with the folder, and copying
necessary files to the folder. There were so many moving parts
and this created race conditions and caused slow test run.

Since v0.4.x, gopls starts to support single file edit and automatically
constructs an internal workspace by walking the directory tree
around the open file. This CL utilizes this new capability.
In each suite, we start testing by starting a new gopls, opening
a single file, and waiting for the gopls to finish the initial package
loading.

This CL introduces Env.onPatternInTrace, which watches the
fake trace output channel, and emits an event when a registered
pattern is observed. This is a hack to wait for the gopls's internal
state to reach to a desirable state - ideally, we want to intercept
all the progress report messages like gopls' regression tests
once it is supported from the language client middleware.
microsoft/vscode-languageserver-node#671

We also identified subtle issues in the existing test setup.

Issue 1: when the code for testing starts (using `vscode-test`'s
`runTests`) we pass the extension development path. It seems like
the vscode instance uses the `main` program specified in `package.json`
and activates it even without us asking. As a result, when we run
tests and call 'activate' again, multiple hover/completion providers
are registered, and vscode returns results from legacy and gopls-based
providers. For example, the completion middleware test was observing
entries from gopls, and goCompletionItemProvider that uses gocode.

We address this issue here by introducing the VSCODE_GO_IN_TEST
environment variable. If it is set, activate will return immediately.
So, tests can control when to register what, and how.

We need this setting in both `launch.json` and `runTest.ts` that's
invoked in CI (`npm run test`)

Issue 2: when the code for testing needs to call `activate`, we
got the extension instance by using `vscode.extensions.getExtension`
and called its `activate`. This was because there is no easy way
to supply sufficiently complete vscode's ExtensionContext.
It turned out, the extension instance vscode.extensions.getExtension
returns is the one built with the `main` program specified in
our `package.json` - that is the webpack'ed one in `dist/goMain.js`.
On the other hand, our debugging depends on pre-webpack versions
in `out/*`. This caused confusion and made debugging near impossible
(breakpoints we set on pre-webpack versions will not be hit because
we are running a different version of extension)!

We don't know if there is a way to teach `vscode-test` to use pre-webpack
version. Maybe this is our misconfiguration in our launch.json and
package.json. For now, we work around this issue by not calling
`activate`. Instead, in this gopls test, we call `buildLanguageClient`
directly. This required some refactoring work in goLanguageServer.ts.

Issue 3: sinon is cool, but stubbing vscode API for channel creation
is too much. While we refactor buildLanguageClient, we made changes
to let the caller supply the output channels.

Issue 4: as `vscode-test` starts the test vscode instance, it also
activates the registered snippets and it interferes with our gopls
completion middleware tests. In test, now we explicitly filter out
the snippet entries.

Issue 5: for some reason, the first entry in the completion middleware
test that expects 'Print' item, the filter text is not set. It can be
a bug, or working as intended (the first item has label === filterText).
Gopls is providing the expected entry. Workaround this issue by inspecting
the label field too.

Updates #655
Updates #832

Change-Id: Ic7088fd551329d1c8f78078ccb24a5f529eec72a
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/266418
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/276714 mentions this issue: test: fix remote attach tests on windows long tests

gopherbot pushed a commit that referenced this issue Dec 10, 2020
@suzmue
test: fix remote attach tests on windows long tests
27a25c4 
The filepaths returned by delve will have '/' as the path separator
even on windows. One of the breakpoint tests was missing this
conversion.

The port was also not correctly set on the disconnect request.

This change does not fix all of the issues, but does keep the tests
from crashing, which should allow us to easier see the test failures.

Updates #832

Change-Id: I829ad6186001f9c665079aa69d99673068a999e8
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/276714
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Suzy Mueller <suzmue@golang.org>
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/277192 mentions this issue: test: increase timeout for extension tests

gopherbot pushed a commit that referenced this issue Dec 10, 2020
@suzmue
test: increase timeout for extension tests
b950da5 
The Workspace Symbols test has been timing out in the long test. The
case where the GOROOT is scanned for symbols as well takes about
15000 on the long tests. Increasing the timeout should make this test
succeed more.

Updates #832

Change-Id: I3bf0f4ccc1b74c1b9e1aa1bca32d8f90f01b2e96
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/277192
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/277552 mentions this issue: test: increase timeouts for windows tests

hyangah added a commit to hyangah/vscode-go that referenced this issue Jul 19, 2022
@hyangah
test/integration/goDebug: catch any error while cleaning up
06ba506 
On windows, sometimes we get Resource Busy errors.
We catch and log the errors gut subsequent file deletion
still can fail. Catch all errors.

For golang#832

Change-Id: I81fe195c9862b2a8f96cbc0bebf9b39f40f6d21d
hyangah added a commit to hyangah/vscode-go that referenced this issue Jul 19, 2022
@hyangah
test/integration: skip tests broken on windows
a0e1425 
Added affectedByIssue832 to clearly indicate the tests
that need to be fixed.

For golang#832
For golang#1788

FORCE RUN CI

Change-Id: Ied7dc9726b33f092ae93a1f11952f97fd1433337
hyangah added a commit to hyangah/vscode-go that referenced this issue Jul 19, 2022
@hyangah
test/integration: skip tests broken on windows
ca8e46c 
Added affectedByIssue832 to clearly indicate the tests
that need to be fixed.

For golang#832
For golang#1788

FORCE RUN CI

Change-Id: Ied7dc9726b33f092ae93a1f11952f97fd1433337
hyangah added a commit to hyangah/vscode-go that referenced this issue Jul 19, 2022
@hyangah
test/integration: skip tests broken on windows
6921fd1 
Added affectedByIssue832 to clearly indicate the tests
that need to be fixed.

For golang#832
For golang#1788

FORCE RUN CI

Change-Id: Ied7dc9726b33f092ae93a1f11952f97fd1433337
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/418356 mentions this issue: test/integration: skip tests broken on windows

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/418355 mentions this issue: test/integration/goDebug: catch any error while cleaning up

gopherbot pushed a commit that referenced this issue Jul 20, 2022
@hyangah
test/integration/goDebug: catch any error while cleaning up
c8598b1 
On windows, sometimes we get Resource Busy errors.
We catch and log the errors gut subsequent file deletion
still can fail. Catch all errors.

For #832

Change-Id: I81fe195c9862b2a8f96cbc0bebf9b39f40f6d21d
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/418355
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
hyangah added a commit to hyangah/vscode-go that referenced this issue Jul 20, 2022
@hyangah
test/integration: skip tests broken on windows
5dd2f19 
Added affectedByIssue832 to clearly indicate the tests
that need to be fixed.

For golang#832
For golang#1788

Change-Id: Ied7dc9726b33f092ae93a1f11952f97fd1433337
gopherbot pushed a commit that referenced this issue Jul 20, 2022
@hyangah
test/integration: skip tests broken on windows
9bb57d8 
Added affectedByIssue832 to clearly indicate the tests
that need to be fixed.

For #832
For #1788

Change-Id: Ied7dc9726b33f092ae93a1f11952f97fd1433337
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/418356
Reviewed-by: Jamal Carvalho <jamal@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
hyangah added a commit to hyangah/vscode-go that referenced this issue Jul 21, 2022
@hyangah
test/integration/goTest: fix Document opened test
8a80a41 
And reenable the test.

The test was broken due to a bug in composing the file Uris for
files in a directory from the directory Uri. path.join uses the
platform specific separator (\ on windows). To ensure the correct
encoding, convert the directory Uri to its fsPath, construct the
file path of the file, and convert the file path to Uri.

For golang#832
For golang#1788

Change-Id: I3dfc44e04f1683b1b862b339b7257b432c3f2241
hyangah added a commit to hyangah/vscode-go that referenced this issue Jul 21, 2022
@hyangah
test/integration/goTest: fix Document opened test
cd4d606 
And reenable the test.

The test was broken due to a bug in composing the file Uris for
files in a directory from the directory Uri. path.join uses the
platform specific separator (\ on windows). To ensure the correct
encoding, convert the directory Uri to its fsPath, construct the
file path of the file, and convert the file path to Uri.

For golang#832
For golang#1788

Change-Id: I3dfc44e04f1683b1b862b339b7257b432c3f2241
hyangah added a commit to hyangah/vscode-go that referenced this issue Jul 21, 2022
@hyangah
test/integration/goTest: fix populateModulePathCache
a6ca693 
And reenable affected tests.

The tests were broken due to a bug in composing the file Uris for
files in a directory from the directory Uri. path.join uses the
platform specific separator (\ on windows). To ensure the correct
encoding, convert the directory Uri to its fsPath, construct the
file path of the file, and convert the file path to Uri.

For golang#832
For golang#1788

Change-Id: I3dfc44e04f1683b1b862b339b7257b432c3f2241
hyangah added a commit to hyangah/vscode-go that referenced this issue Jul 21, 2022
@hyangah
src/goTest/resolve: fix nested packageDisplayMode handling on win32
d247c4b 
To use path module, use Uri.fsPath, not Uri.path.

For golang#832
For golang#1788

Change-Id: Ic07e19f2f83cc4b05b3be750de4be7bcdb6e6656
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/418547 mentions this issue: src/goTest/resolve: fix nested packageDisplayMode handling on win32

@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/418545 mentions this issue: test/integration/goTest: fix populateModulePathCache

hyangah added a commit to hyangah/vscode-go that referenced this issue Jul 21, 2022
@hyangah
src/goTest/resolve: fix nested packageDisplayMode handling on win32
55f5d43 
To use path module, use Uri.fsPath, not Uri.path.

For golang#832
For golang#1788

Change-Id: Ic07e19f2f83cc4b05b3be750de4be7bcdb6e6656
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/418779 mentions this issue: test/integration/goExplorer: fix 'env tree items' test

gopherbot pushed a commit that referenced this issue Jul 21, 2022
@hyangah
test/integration/goTest: fix populateModulePathCache
47f6dfa 
And reenable affected tests.

The tests were broken due to a bug in composing the file Uris for
files in a directory from the directory Uri. path.join uses the
platform specific separator (\ on windows). To ensure the correct
encoding, convert the directory Uri to its fsPath, construct the
file path of the file, and convert the file path to Uri.

For #832
For #1788

Change-Id: I3dfc44e04f1683b1b862b339b7257b432c3f2241
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/418545
Reviewed-by: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopherbot pushed a commit that referenced this issue Jul 22, 2022
@hyangah
src/goTest/resolve: fix nested packageDisplayMode handling on win32
d2afd7d 
To use path module, use Uri.fsPath, not Uri.path.

For #832
For #1788

Change-Id: Ic07e19f2f83cc4b05b3be750de4be7bcdb6e6656
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/418547
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
@gopherbot
Copy link
Collaborator

Change https://go.dev/cl/419110 mentions this issue: test/integration/extension: disable all "Test Completion Snippets *"

gopherbot pushed a commit that referenced this issue Jul 22, 2022
@hyangah
test/integration/goExplorer: fix 'env tree items' test
cbecbf8 
use path.join for file path

For #832

Change-Id: I03817df9ee3cbdff6b5d537377112548a6055d1b
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/418779
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
gopherbot pushed a commit that referenced this issue Jul 22, 2022
@hyangah
test/integration/extension: disable all "Test Completion Snippets *"
5bf7237 
Skip "Test Completion Snippets For Functions" on windows.

This should've been included in go.dev/cl/418356

For #832

Change-Id: Ieff2faa0bfd9154dcc834a30e0dc2259c4fa3b20
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/419110
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Nooras Saba‎ <saba@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
vscode-go/backlog
Development

No branches or pull requests
2 participants
@hyangah @gopherbot