Skip to content

Actually compile Go VM tests; halve CircleCI test time#2589

Merged
hugelgupf merged 27 commits into
u-root:mainfrom
hugelgupf:make-arm64-tests-work
Jan 2, 2023
Merged

Actually compile Go VM tests; halve CircleCI test time#2589
hugelgupf merged 27 commits into
u-root:mainfrom
hugelgupf:make-arm64-tests-work

Conversation

@hugelgupf
Copy link
Copy Markdown
Member

@hugelgupf hugelgupf commented Dec 29, 2022 •
edited
Loading

This initially worked, but almost all VM Go unit testing broken when Gobusybox was made the default by @MDr164 in May 2022, as this comment indicates. All arm64 tests and and most if not all x86 VM-based Go tests (like pkg/ipmi, pkg/mount, pkg/smbios) haven't been running.

Also, we don't seem to notice that the tests aren't running in the first place, but I'm dealing with that later.

End result of this PR:

  • CircleCI presubmit now runs in ~15 minutes instead of 30 minutes
  • Go unit VM tests work again (amd64 & arm64)
  • 1.37% coverage increase
  • this list of broken tests fixed
  • Couple of tests disabled, but not really relevant.

Before:

...
integration.go:145: 2022/09/27 10:58:36 serial: 1970/01/01 00:01:26 Failed to start "/testdata/tests/github.com/u-root/u-root/pkg/boot/menu/menu.test": fork/exec /testdata/tests/github.com/u-root/u-root/pkg/boot/menu/menu.test: exec format error~
...

After:

...
integration.go:145: 2022/12/28 19:07:22 serial: === RUN   TestDMIDecode~
...

This is a screenshot of test-integration-arm64 of 3 months ago, not running but passing:

Screenshot 2022-12-28 19 10 58

This is a screenshot of test running pkg/ulog test, 3 months ago, not running but passing:

image

rminnich
rminnich previously approved these changes Dec 29, 2022
View reviewed changes
@hugelgupf hugelgupf changed the title Compile Go VM tests for target architecture Actually compile Go VM tests Dec 29, 2022
@hugelgupf
Copy link
Copy Markdown
Member Author

hugelgupf commented Dec 29, 2022 •
edited
Loading

Currently broken:

on x86

  • pkg/smbios (type 9 not found - afaict that's expected in QEMU, so changed that test)
  • pkg/mount/block (kernel crashing in NVMe stuff -- solved by -machine q35)
  • pkg/ipmi (open /dev/ipmi0: no such file or directory (with kernel in our Docker image) -- upgrading kernel -- and ipmi_vm_test.go:47: i.WatchdogRunning() = true, not false (with 6.0 kernel and IPMI options) -- afaict the watchdog is not running in QEMU unless activated by someone? changing expectation to not running in test.)
  • cmds/exp/pox (times out under race, like many VM tests -- needs to be disabled)
  • pkg/gpio (failed after upgrade to 6.0 kernel at first)
  • pkg/efivarfs (inappropriate ioctl for device -- tmpfs does not implement fileattr_get / FS_IOC_GETFLAGS & FS_IOC_SETFLAGS -- 6.0 kernel does with CONFIG_TMPFS_XATTR is enabled)

on arm64

  • pkg/efivarfs (varfs_test.go:367: Want: permission denied, Got: ioctl /tmp/TestRemoveno_permission1675683738/001/TestVar-bc54d3fb-ed45-462d-9df8-b9f736228350: inappropriate ioctl for device)
  • pkg/boot/fit.TestLoad (depends on current time, but aarch64 VMs start at 1970-01-01 by default)
  • cmds/exp/cbmem (Error: test for "github.com/u-root/u-root/cmds/exp/cbmem" exited early: signal: killed)
  • pkg/rtc (rtc_linux_test.go:57: OpenRTC got: no RTC device found; want nil)
  • I suspect after this point some special characters messed up the JSON parsing, so test pass/fail is unreliable.

Crash log from pkg/mount/block for documentation:

    integration.go:140: 2022/12/29 22:57:41 serial: Run /init as init process~                                                                                      
    integration.go:140: 2022/12/29 22:57:41 serial: 2022/12/30 06:57:41 Welcome to u-root!~                                                                         
    integration.go:140: 2022/12/29 22:57:41 serial:                               _~                                                                                
    integration.go:140: 2022/12/29 22:57:41 serial:    _   _      _ __ ___   ___ | |_~                                                                              
    integration.go:140: 2022/12/29 22:57:41 serial:   | | | |____| '__/ _ \ / _ \| __|~
    integration.go:140: 2022/12/29 22:57:41 serial:   | |_| |____| | | (_) | (_) | |_~
    integration.go:140: 2022/12/29 22:57:41 serial:    \__,_|    |_|  \___/ \___/ \__|~                                                                             
    integration.go:140: 2022/12/29 22:57:41 serial: ~  
    integration.go:140: 2022/12/29 22:57:42 serial: irq 11: nobody cared (try booting with the "irqpoll" option)~                                                   
    integration.go:140: 2022/12/29 22:57:42 serial: CPU: 0 PID: 0 Comm: swapper Not tainted 6.0.0 #1~                                                               
    integration.go:140: 2022/12/29 22:57:42 serial: Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-debian-1.16.0-4 04/01/2014~                  
    integration.go:140: 2022/12/29 22:57:42 serial: Call Trace:~                                                                                                    
    integration.go:140: 2022/12/29 22:57:42 serial:  <IRQ>~                                                                                                         
    integration.go:140: 2022/12/29 22:57:42 serial:  dump_stack_lvl+0x19/0x27~                                                                                      
    integration.go:140: 2022/12/29 22:57:42 serial:  __report_bad_irq+0x2e/0xa3~                                                                                    
    integration.go:140: 2022/12/29 22:57:42 serial:  note_interrupt.cold+0xa/0x5f~                                                                                  
    integration.go:140: 2022/12/29 22:57:42 serial:  handle_irq_event+0x59/0x70~                                                                                    
    integration.go:140: 2022/12/29 22:57:42 serial:  handle_fasteoi_irq+0x63/0x1b0~                                                                                 
    integration.go:140: 2022/12/29 22:57:42 serial:  __common_interrupt+0x37/0x90~                                                                                  
    integration.go:140: 2022/12/29 22:57:42 serial:  common_interrupt+0x36/0x90~                                                                                    
    integration.go:140: 2022/12/29 22:57:42 serial:  asm_common_interrupt+0x22/0x40~                                                                                
    integration.go:140: 2022/12/29 22:57:42 serial: RIP: 0010:__do_softirq+0x65/0x1de~                                                                              
    integration.go:140: 2022/12/29 22:57:42 serial: Code: 2c ff f7 ff ff 89 74 24 14 81 05 fa af 44 00 00 01 00 00 c7 44 24 10 0a 00 00 00 66 c7 05 a1 8e 44 00 00 0
0 fb b8 ff ff ff ff <41> 0f bc c7 41 89 c4 41 83 c4 01 74 70 49 c7 c3 c0 8c e1 8e 48 98~                                                                            
    integration.go:140: 2022/12/29 22:57:42 serial: RSP: 0018:ffff94c640003fa0 EFLAGS: 00000202~                                                                    
    integration.go:140: 2022/12/29 22:57:42 serial: RAX: 00000000ffffffff RBX: ffffffff8ea03e08 RCX: ffffffffffffffff~                                              
    integration.go:140: 2022/12/29 22:57:42 serial: RDX: 0000000000000000 RSI: 0000000004200002 RDI: ffff8c1101130000~                                              
    integration.go:140: 2022/12/29 22:57:42 serial: RBP: 0000000000000001 R08: ffffffffede9af7e R09: 0000000022a1b163~                                              
    integration.go:140: 2022/12/29 22:57:42 serial: R10: 0000000000000200 R11: 0000000000000000 R12: 0000000000000000~                                              
    integration.go:140: 2022/12/29 22:57:42 serial: R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000200~                                              
    integration.go:140: 2022/12/29 22:57:42 serial:  sysvec_apic_timer_interrupt+0x63/0x80~                                                                         
    integration.go:140: 2022/12/29 22:57:42 serial:  </IRQ>~                                                                                                        
    integration.go:140: 2022/12/29 22:57:42 serial:  <TASK>~                                                                                                        
    integration.go:140: 2022/12/29 22:57:42 serial:  asm_sysvec_apic_timer_interrupt+0x16/0x20~                                                                     
    integration.go:140: 2022/12/29 22:57:42 serial: RIP: 0010:default_idle+0x13/0x20~                                                                               
    integration.go:140: 2022/12/29 22:57:42 serial: Code: ff e9 7b ff ff ff 0f 1f 44 00 00 e9 7c ff ff ff e8 32 98 ff ff cc cc 8b 05 b2 00 a6 00 85 c0 7e 07 0f 00 2
d 1f 92 45 00 fb f4 <c3> cc cc cc cc 0f 1f 84 00 00 00 00 00 48 8b 04 25 40 c0 a4 8e 80~                                                                            
    integration.go:140: 2022/12/29 22:57:42 serial: RSP: 0018:ffffffff8ea03eb0 EFLAGS: 00000246~                                                                    
    integration.go:140: 2022/12/29 22:57:42 serial: RAX: 0000000000000000 RBX: ffffffff8ea26600 RCX: 0000000000000000~                                              
    integration.go:140: 2022/12/29 22:57:42 serial: RDX: 4000000000000000 RSI: 0000000000000000 RDI: 00000000000042ec~                                              
    integration.go:140: 2022/12/29 22:57:42 serial: RBP: 0000000000000000 R08: ffffffffede9af7e R09: 0000000022a1b163~                                              
    integration.go:140: 2022/12/29 22:57:42 serial: R10: ffff8c11024ca010 R11: ffff8c110246a148 R12: 0000000000000000~                                              
    integration.go:140: 2022/12/29 22:57:42 serial: R13: 0000000000000000 R14: ffffffff8ea26118 R15: 0000000000000000~                                              
    integration.go:140: 2022/12/29 22:57:42 serial:  default_idle_call+0x23/0x30~                                                                                   
    integration.go:140: 2022/12/29 22:57:42 serial:  do_idle+0x160/0x1c0~                                                                                           
    integration.go:140: 2022/12/29 22:57:42 serial:  cpu_startup_entry+0xa/0x10~                                                                                    
    integration.go:140: 2022/12/29 22:57:42 serial:  rest_init+0x9a/0xa0~                                                                                           
    integration.go:140: 2022/12/29 22:57:42 serial:  arch_call_rest_init+0x5/0xa~
    integration.go:140: 2022/12/29 22:57:42 serial:  start_kernel+0x5cc/0x5f5~
    integration.go:140: 2022/12/29 22:57:42 serial:  secondary_startup_64_no_verify+0xe0/0xeb~
    integration.go:140: 2022/12/29 22:57:42 serial:  </TASK>~
    integration.go:140: 2022/12/29 22:57:42 serial: handlers:~                                                                                                      
    integration.go:140: 2022/12/29 22:57:42 serial: [<00000000f0549d9d>] nvme_irq~                                                                                  
    integration.go:140: 2022/12/29 22:57:42 serial: [<00000000f0549d9d>] nvme_irq~                                                                                  
    integration.go:140: 2022/12/29 22:57:42 serial: [<00000000dd045a61>] vp_interrupt~                                                                              
    integration.go:140: 2022/12/29 22:57:42 serial: Disabling IRQ #11~

@hugelgupf
Compile Go VM tests for target architecture
9113e07 
This never actually worked.

Before:

    ...
    integration.go:145: 2022/09/27 10:58:36 serial: 1970/01/01 00:01:26 Failed to start "/testdata/tests/github.com/u-root/u-root/pkg/boot/menu/menu.test": fork/exec /testdata/tests/github.com/u-root/u-root/pkg/boot/menu/menu.test: exec format error~
    ...

After:

    ...
    integration.go:145: 2022/12/28 19:07:22 serial: === RUN   TestDMIDecode~
    ...

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Give arm64 tests 20 minutes
c0a4669 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Skip VM test during race tests
27d138c 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Improve GET_KERNEL_QEMU script
84553d0 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
smbios: disable table type 9 tests
143a3d0 
QEMU does not create SMBIOS type 9 tables.

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Fix pkg/mount/block VM crash
2e9f75b 
Crash log in u-root#2589.

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
ipmi: make VM test error msgs readable
e21d0ac 
https://google.github.io/styleguide/go/decisions#got-before-want

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
ipmi VM test: expect watchdog not to be running
cfe1cfa 
I'm not sure if it's supposed to be running or not. But it's not.

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf hugelgupf force-pushed the make-arm64-tests-work branch from daf9337 to 5001f1d Compare December 30, 2022 08:31
@rminnich
Copy link
Copy Markdown
Member

rminnich commented Dec 31, 2022

do we need to revert #1708 and just start over with multiarchitecture test?

@hugelgupf
Copy link
Copy Markdown
Member Author

hugelgupf commented Dec 31, 2022

do we need to revert #1708 and just start over with multiarchitecture test?

I'm just gonna fix them one by one (or disable the ones that can't be fixed because they don't make sense on arm64), and then make sure it can't happen again.

Couple more things that need to happen that I'm checking out right now:

  • it needs to be easier to run all VM tests as multi-architecture. right now, we only run the ones in integration/ as multi-arch. pkg/{ipmi,mount,smbios,ulog,cmos} all run x86 only (and were broken, too).
  • vmtest itself needs tests to ensure that it actually works, and that failing tests are reported correctly. breaking it out into its own project and having its own test suite there IMO is the way to go.

@hugelgupf hugelgupf force-pushed the make-arm64-tests-work branch from 4af2db4 to 15dd8b7 Compare December 31, 2022 21:29
@hugelgupf
Upgrade amd64 test to a 6.0 kernel
ac53484 
It's compilable with gcc 11 and 12... the 5.10 kernel wasn't anymore.

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Upgrade amd64 to v5.2.0 images using v6.0 kernel
075d1b5 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
pox VM test: don't run on race
241c037 
race test config does not set timeouts and other env vars; many of our
VM tests are disabled in race mode. Doesn't make sense to run them there
anyway.

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf hugelgupf force-pushed the make-arm64-tests-work branch from 15dd8b7 to 241c037 Compare December 31, 2022 21:30
@hugelgupf
efivarfs: use corrrect constant name for immutable bit
d96bb68 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
efivarfs: more detail in error message
bf68929 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Add CONFIG_TMPFS_XATTR; upgrade arm64 Linux to 6.0
5de5ba1 
pkg/efivarfs requires CONFIG_TMPFS_XATTR. It's not available until
arm64.

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Upgrade test-image-arm64 to v5.2.0 (with 6.0 kernel)
7b30461 
And CONFIG_TMPFS_XATTR for pkg/efivarfs

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
efivarfs: re-enable permissions tests on x86 VMs
70efa8c 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Add RTC configs to kernel
5f6f403 
pkg/boot/fit does signature checks that depend on time. Bad idea, but
for now, let's make time work.

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
gotests: set up RTC
ebcde0c 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf hugelgupf force-pushed the make-arm64-tests-work branch 3 times, most recently from caa4082 to 4b72a90 Compare January 1, 2023 06:10
@hugelgupf
Disable RSDP test on arm64
151b32f 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
arm64: disable slow tests
a821bfe 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf hugelgupf force-pushed the make-arm64-tests-work branch from 4b72a90 to e82cca0 Compare January 1, 2023 06:30
@hugelgupf
Use integration-template for arm64
93c4594 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Split testing pkg and cmds, to reduce overall test time
77a7bb1 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Remove test that was never run anyway
451c49d 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf hugelgupf force-pushed the make-arm64-tests-work branch 2 times, most recently from dc72cc9 to ad1e040 Compare January 1, 2023 07:43
@hugelgupf
test: run at most one test binary at a time
3a84ed0 
Use go test -p flag (documented in `go help build`) for this.

But set GOMAXPROCS back to a useful value to allow concurrency.

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Remove unnecessary env var setting
0a5cf75 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Skip IPMI test in race detector
7e68fff 
Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf hugelgupf force-pushed the make-arm64-tests-work branch from ad1e040 to 7e68fff Compare January 1, 2023 07:48
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 1, 2023 •
edited
Loading

Codecov Report

Base: 73.54% // Head: 74.75% // Increases project coverage by +1.20% 🎉

Coverage data is based on head (bcbfa88) compared to base (656d490).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2589      +/-   ##
==========================================
+ Coverage   73.54%   74.75%   +1.20%     
==========================================
  Files         406      408       +2     
  Lines       41209    41335     +126     
==========================================
+ Hits        30309    30900     +591     
+ Misses      10900    10435     -465
Impacted Files Coverage Δ
pkg/efivarfs/fs.go 85.00% <100.00%> (+10.00%) ⬆️
pkg/vmtest/gotest.go 78.57% <100.00%> (+11.90%) ⬆️
pkg/efivarfs/varfs.go 59.20% <0.00%> (-3.20%) ⬇️
pkg/ldd/ldd_unix.go 79.79% <0.00%> (-1.02%) ⬇️
...g/boot/multiboot/internal/trampoline/trampoline.go 100.00% <0.00%> (ø)
pkg/boot/linux/load_linux_arm64.go 0.00% <0.00%> (ø)
pkg/smbios/type4_processor_information.go 96.36% <0.00%> (+0.42%) ⬆️
pkg/vmtest/integration.go 64.81% <0.00%> (+0.61%) ⬆️
cmds/core/date/date.go 91.60% <0.00%> (+0.76%) ⬆️
cmds/core/ip/ops_linux.go 79.82% <0.00%> (+0.87%) ⬆️
... and 26 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@hugelgupf hugelgupf changed the title Actually compile Go VM tests Actually compile Go VM tests; halve CircleCI test time Jan 1, 2023
@hugelgupf
More resources to speed up arm64 tests
bcbfa88 
Improves running time from 19 mins to 14 mins

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf hugelgupf force-pushed the make-arm64-tests-work branch from 0c123c5 to bcbfa88 Compare January 1, 2023 08:46
rminnich
rminnich approved these changes Jan 2, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@rminnich rminnich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amazing.

@rminnich rminnich added the Awaiting author Waiting for new changes or feedback for author. label Jan 2, 2023
@hugelgupf hugelgupf merged commit ff9c888 into u-root:main Jan 2, 2023
@hugelgupf hugelgupf deleted the make-arm64-tests-work branch January 2, 2023 06:01
hugelgupf added a commit that referenced this pull request Jan 2, 2023
@hugelgupf
Fix pkg/mount/block VM crash
f339124 
Crash log in #2589.

Signed-off-by: Chris Koch <chrisko@google.com>
hugelgupf added a commit to hugelgupf/vmtest that referenced this pull request Jan 2, 2023
@hugelgupf
Cross-compilation that actually works
d9fe328 
Fix same issue as u-root/u-root#2589

Signed-off-by: Chris Koch <chrisko@google.com>
hugelgupf added a commit to hugelgupf/vmtest that referenced this pull request Jan 2, 2023
@hugelgupf
Cross-compilation that actually works
4d0de5b 
Fix same issue as u-root/u-root#2589

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf hugelgupf mentioned this pull request Feb 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rminnich rminnich rminnich approved these changes

Assignees

No one assigned

Labels

Awaiting author Waiting for new changes or feedback for author.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hugelgupf @rminnich