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

Open
stamblerre opened this issue Jul 28, 2020 · 8 comments
Open

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

stamblerre opened this issue Jul 28, 2020 · 8 comments
Assignees
Labels
Milestone

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jul 28, 2020

@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
@findleyr
Copy link

@findleyr findleyr commented Jul 28, 2020

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

@stamblerre stamblerre commented Jul 28, 2020

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 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

@findleyr findleyr commented Jul 29, 2020

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

@stamblerre stamblerre commented Jul 29, 2020

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

@gopherbot gopherbot commented Jul 29, 2020

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

@findleyr
Copy link

@findleyr findleyr commented Jul 29, 2020

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

@stamblerre stamblerre commented Jul 29, 2020

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.