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/gopls: set up govim regression tests to run at govim@main #40451

Closed
stamblerre opened this issue Jul 28, 2020 · 11 comments
Closed

x/tools/gopls: set up govim regression tests to run at govim@main #40451

stamblerre opened this issue Jul 28, 2020 · 11 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@stamblerre
Copy link
Contributor

@heschik has mentioned that, now that we have our own regression tests, it feels like it's time to stop relying on the govim tests to run on each CL.

When the govim tests fail, we have started rewriting them into our own regression tests, which seems like the right approach. I think the govim tests are a little brittle - particularly because pinning a govim version means we don't get updates as we change gopls. For example, https://golang.org/cl/227033 can't be submitted because we need to bump the version of govim in our CI, but we can't bump the version because of other failures.

It is nice to be able to catch things earlier, so maybe we could have the govim tests running for gopls at master and present those results in a dashboard?

/cc @findleyr

@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Jul 28, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jul 28, 2020
@findleyr
Copy link
Contributor

FWIW, we can easily set the up to run as a post-submit on master. Not sure about getting them in a public dashboard, however.

@stamblerre
Copy link
Contributor Author

Post-submit sounds OK to me, as long as we have a good way to update to later versions of govim, etc. I'd just like to be able to submit breaking CLs without making our govim tests useless.

@myitcv
Copy link
Member

myitcv commented Jul 29, 2020

FWIW we have a daily build of govim@main against gopls@master:

https://github.com/govim/govim/actions?query=event%3Aschedule

@findleyr
Copy link
Contributor

I'd just like to be able to submit breaking CLs without making our govim tests useless.

You mean 'useless' in the sense that they're already failing? That will still happen if you submit a breaking CL... but I think you mean just that it would be good to keep things more closely in sync. As long as you don't mind getting an email for every failure until things are fixed, I'll set them up to run gopls@master and govim@master.

@stamblerre
Copy link
Contributor Author

You mean 'useless' in the sense that they're already failing? That will still happen if you submit a breaking CL... but I think you mean just that it would be good to keep things more closely in sync.

Yeah exactly. Having them both run at master would be great, so then we can send a PR to govim whenever we make a breaking change.

@stamblerre stamblerre changed the title x/tools/gopls: consider disabling govim regression tests x/tools/gopls: set up govim regression tests to run at master Jul 29, 2020
@stamblerre stamblerre changed the title x/tools/gopls: set up govim regression tests to run at master x/tools/gopls: set up govim regression tests to run at govim master Jul 29, 2020
@stamblerre stamblerre changed the title x/tools/gopls: set up govim regression tests to run at govim master x/tools/gopls: set up govim regression tests to run at govim@main Jul 29, 2020
@gopherbot
Copy link

Change https://golang.org/cl/245539 mentions this issue: gopls/integration/govim: enable running at main

@findleyr
Copy link
Contributor

BTW, our current CI is passing at govim@main, because we're running with -short. As part of switching to a different CI system, I think we can run all tests (not just --short), which will pull in some failing tests.

@stamblerre
Copy link
Contributor Author

Agreed - let's do that.

gopherbot pushed a commit to golang/tools that referenced this issue Jul 29, 2020
We're going to switch to running govim tests at main as post-submit CI
rather than presubmit, and will also switch to running them via Kokoro
using the run_local script rather than cloud build.

Enable this by changing the semantics of run_local.sh to default to
main.

For golang/go#40451

Change-Id: I9c311dea8326a36a3f8335eddbfae0ce7f02f6bf
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245539
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/247681 mentions this issue: gopls/integration/govim: use docker to build gopls in run_local.sh

gopherbot pushed a commit to golang/tools that referenced this issue Aug 10, 2020
Kokoro build runners have an old version of the Go command. Update the
run_local.sh script that they use to build in Docker.

For golang/go#40451

Change-Id: I89fc422e6824045ffdb610fe0f9a106c6cdb875e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/247681
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@stamblerre
Copy link
Contributor Author

@findleyr: Can this be closed now?

@findleyr
Copy link
Contributor

Yup!

@golang golang locked and limited conversation to collaborators Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants