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

cmd/link: short tests are not short enough #26470

Open
rsc opened this issue Jul 19, 2018 · 5 comments
Open

cmd/link: short tests are not short enough #26470

rsc opened this issue Jul 19, 2018 · 5 comments
Labels
NeedsInvestigation Testing
Milestone
Backlog

Comments

@rsc
Copy link
Contributor

@rsc rsc commented Jul 19, 2018

Running all.bash (which does go test -short std cmd), my target is usually that individual tests should run in under 1 second.

The cmd/link test takes 3.4s on my system. Can we make it take less time?

cmd/link is implicitly tested by pretty much every other test in the system, so it's hard to believe it needs to do a ton in the -short test.

/cc @aclements
@rsc rsc added this to the Go1.12 milestone Jul 19, 2018
@rsc
Copy link
Contributor Author

@rsc rsc commented Jul 19, 2018

I missed that cmd/link/internal/ld takes an additional 12.9 seconds. That's even worse.

@rsc rsc mentioned this issue Jul 19, 2018
Open
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 20, 2018

Change https://golang.org/cl/125295 mentions this issue: cmd/link: skip a couple of DWARF tests in short mode

gopherbot pushed a commit that referenced this issue Jul 20, 2018
@thanm
cmd/link: skip a couple of DWARF tests in short mode
f4c787b 
Rejigger the DWARF tests to ensure that they run in a reasonable
amount of time in short mode, particularly the "abstract origin
sanity" testpoints.

Updates #26470

Change-Id: Idae9763ac20ea999fa394595aacfcd1e271293ae
Reviewed-on: https://go-review.googlesource.com/125295
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
@ysmolsky
Copy link
Member

@ysmolsky ysmolsky commented Sep 18, 2018 •
edited

Takes about 1.4 sec on laptop mac:

% go test cmd/link -v -short
=== RUN   TestDWARF
=== RUN   TestDWARF/testprog
=== RUN   TestDWARF/testprogcgo
--- PASS: TestDWARF (1.30s)
    --- PASS: TestDWARF/testprog (0.39s)
    --- PASS: TestDWARF/testprogcgo (0.78s)
=== RUN   TestDWARFiOS
--- SKIP: TestDWARFiOS (0.00s)
    dwarf_test.go:159: skipping in short mode
=== RUN   TestLargeSymName
--- PASS: TestLargeSymName (0.00s)
=== RUN   TestIssue21703
--- PASS: TestIssue21703 (0.14s)
=== RUN   TestLargeText
--- SKIP: TestLargeText (0.00s)
    linkbig_test.go:24: Skipping large text section test in short mode or on amd64
PASS
ok  	cmd/link	1.448s

Can we close it?

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 7, 2018

Change https://golang.org/cl/153258 mentions this issue: cmd/link: use SeekPC in testDWARF

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 7, 2018

Change https://golang.org/cl/153259 mentions this issue: cmd/link/internal/ld: run tests in parallel

gopherbot pushed a commit that referenced this issue Dec 8, 2018
@ianlancetaylor
cmd/link/internal/ld: run tests in parallel
353795c 
Also skip TestNooptCgoBuild in short mode.

Also fix a couple of obscure constants to use values named in
cmd/internal/dwarf.

This brings the time of the cmd/link/internal/ld tests down to about 1
second on my laptop.

Updates #26470

Change-Id: I71c896f30fd314a81d9090f1b6d02edc4174a808
Reviewed-on: https://go-review.googlesource.com/c/153259
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@andybons andybons removed this from the Go1.12 milestone Feb 12, 2019
@andybons andybons added this to the Go1.13 milestone Feb 12, 2019
gopherbot pushed a commit that referenced this issue Feb 26, 2019
@ianlancetaylor
cmd/link: use SeekPC in testDWARF
e609bd3 
This makes the tests slightly faster, though the bulk of the time is
still spent building the test programs.

Also run some tests in parallel.

Updates #26470

Change-Id: Ia5ec2b99831d69c426b43dbab80613aa03e705f5
Reviewed-on: https://go-review.googlesource.com/c/153258
Reviewed-by: Austin Clements <austin@google.com>
@andybons andybons removed this from the Go1.13 milestone Jul 8, 2019
@andybons andybons added this to the Go1.14 milestone Jul 8, 2019
@rsc rsc removed this from the Go1.14 milestone Oct 9, 2019
@rsc rsc added this to the Backlog milestone Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Testing
Projects
None yet
Milestone
Backlog
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
@rsc @ysmolsky @andybons @bcmills @gopherbot