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

Jump to bottom

x/build,x/pkgsite: enable tests of x/pkgsite in build.golang.org #59347

Closed
hyangah opened this issue Mar 31, 2023 · 18 comments
Closed

x/build,x/pkgsite: enable tests of x/pkgsite in build.golang.org #59347

hyangah opened this issue Mar 31, 2023 · 18 comments
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done. pkgsite
Milestone
pkgsite/backlog

Comments

@hyangah
Copy link
Contributor

hyangah commented Mar 31, 2023 •
edited

cmd/pkgsite works for go documentation browsing on users machines and replaces the traditional godoc.

Include x/pkgsite to build.golang.org and enable relevant tests in platforms users will use to run cmd/pkgsite.

Subtasks
@bcmills bcmills added this to the Unreleased milestone Mar 31, 2023
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 31, 2023
@findleyr
Copy link
Contributor

findleyr commented Apr 6, 2023

CC @golang/release

Should this issue be moved to x/build?

@bcmills
Copy link
Contributor

bcmills commented Apr 6, 2023

There are probably two parts: (1) configure x/build to run the tests for the repo, and (2) fix any tests that are discovered to be already failing. Step (1) is x/build, but step (2) is x/pkgsite.

@bcmills bcmills changed the title x/pkgsite: enable tests in build.golang.org x/build,x/pkgsite: enable tests of x/pkgsite in build.golang.org Apr 6, 2023
@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Apr 6, 2023
@findleyr findleyr modified the milestones: Unreleased, pkgsite/later Apr 6, 2023
@dmitshur
Copy link
Contributor

dmitshur commented May 9, 2023

@hyangah Would you like us to help with the x/build part of this issue? Otherwise most of the work here is to ensure x/pkgsite tests will be passing after a builder is running.

@hyangah
Copy link
Contributor Author

hyangah commented Jun 28, 2023

@dmitshur Yes, please. We will track down broken tests and builds once they appear in the builder.

@gopherbot
Copy link

Change https://go.dev/cl/506875 mentions this issue: repos: enable build of x/pkgsite

gopherbot pushed a commit to golang/build that referenced this issue Jun 28, 2023
@hyangah
repos: enable build of x/pkgsite
7c638f1 
Updates golang/go#59347

Change-Id: I7760607a57b5073730ec3627f15ddde2cc15d244
Reviewed-on: https://go-review.googlesource.com/c/build/+/506875
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
@cagedmantis
Copy link
Contributor

The coordinator has been deployed. x/pkgsite are running.

@dmitshur
Copy link
Contributor

It's also running in LUCI, finding 636 passing tests and 7 failing tests on Windows.

@gopherbot
Copy link

Change https://go.dev/cl/506977 mentions this issue: internal/vuln: fix tests on windows

gopherbot pushed a commit to golang/pkgsite that referenced this issue Jun 29, 2023
@hyangah
internal/vuln: fix tests on windows
caabbd6 
Specify test vuln db data in a platform-independent way.

This CL fixes a path that led to runtime panic shown in
https://build.golang.org/log/eecc6c289819b502bc5449a6af26610092d24a07
And adds comments to clarify Client.byIDs - its return
value can contain nil values if there are no matching
osv entries in the DB.

Updates golang/go#59347

Change-Id: I10a6be8bb1e0bef4f8be204c47591bf20a3480f3
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/506977
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
@hyangah hyangah mentioned this issue Jul 7, 2023
Open
@findleyr
Copy link
Contributor

findleyr commented Jul 7, 2023 •
edited

One problem to solve is the Trybot-Result label. Currently, this is still set by Kokoro.

We should probably do one of the following:

  • create a separate label for Kokoro
  • move dockerized tests to LUCI

@heschi
Copy link
Contributor

heschi commented Jul 12, 2023

We've created the label.

@findleyr
Copy link
Contributor

Kokoro is updated to write to the new label. Submit requirements were not modified, so we still only need the TryBots to succeed in order to submit. How do folks feel about that? Should we continue to require that Kokoro CI pass before submitting?

@findleyr
Copy link
Contributor

Looks like it's working: https://go.dev/cl/509695 (which could use a +1...)

I think we should require kokoro-CI for submission though, since so many tests do not run on TryBots: https://go.dev/cl/509476

@heschi
Copy link
Contributor

heschi commented Jul 18, 2023

What's left to do here?

@dmitshur dmitshur added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 18, 2023
@hyangah
Copy link
Contributor Author

hyangah commented Jul 19, 2023

Looks like

@heschi
Copy link
Contributor

heschi commented Jul 19, 2023

Sounds like it's done from the release team's perspective, marking as such.

@heschi heschi removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 19, 2023
@matloob
Copy link
Contributor

matloob commented Sep 1, 2023

The tests now pass on all platforms they run on, except for android but only at Go 1.20. Go 1.21 has the benefit of https://go.dev/cl/472096 which fixed issues with GOROOT installation on the device. But Go 1.20 doesn't so it can't find the compile tool, and isn't able to load template files from tests.

I think we should just disable the pkgsite tests on Go 1.20 and then mark this as done.

@gopherbot
Copy link

Change https://go.dev/cl/524947 mentions this issue: dashboard: disable pkgsite for pre-1.21 android builders

gopherbot pushed a commit to golang/build that referenced this issue Sep 1, 2023
@matloob
dashboard: disable pkgsite for pre-1.21 android builders
c0b6c31 
The pkgsite tests need CL 472096, released in 1.21 to run properly.
Don't run them on 1.20 and earlier.

For golang/go#59347
For golang/go#61209

Change-Id: I32ba9652401db3b1fb9f23c9777e4514332abaef
Reviewed-on: https://go-review.googlesource.com/c/build/+/524947
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@dmitshur
Copy link
Contributor

x/pkgsite is all green on both post-submit dashboards (new and old). Thanks all. Closing as done.

@bcmills bcmills mentioned this issue Feb 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done. pkgsite
Projects
Go Release
Archived in project
Milestone
pkgsite/backlog
Development

No branches or pull requests
8 participants
@cagedmantis @dmitshur @hyangah @bcmills @gopherbot @matloob @heschi @findleyr