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

cmd/compile: split codegen tests into files (one for each arch) #24077

Closed
ALTree opened this issue Feb 23, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@ALTree
Copy link
Member

commented Feb 23, 2018

cmd/compile/internal/gc/asm_test.go contains code generation tests for all architectures. The tests are grouped in very long arrays, one for each architecture. For example the amd64 codegen tests are inside an array named linuxAMD64Tests spanning ~1500 lines of code (from 279 line to line 1770). All the architectures arrays are one after the other.

Working on the file is slightly annoying because when writing, reading or grepping through it you can never tell inside which architecture's array you are, so I find myself constantly scrolling/jumping back in the file until I find the start of the array, where I can read the name of the variable that holds it and determine which architecture tests I'm looking at.

Reviewing changes to the file is also difficult, because a reviewer will need to expand potentially hundreds of unchanged lines in the diff just to be sure that a new test is being added to the rigth array.

I propose we split the tests into files, one for each architecture. We can keep asm_test.go for the non-arch-specific tests, and for the helper functions and the codegen driver, and put the arrays with the tests in their own files named codegen_amd64_test.go, and so on.

@randall77

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2018

That sounds fine.

@ALTree

This comment has been minimized.

Copy link
Member Author

commented Feb 23, 2018

Sweet. Will send CL soon.

@gopherbot

This comment has been minimized.

Copy link

commented Feb 26, 2018

Change https://golang.org/cl/97095 mentions this issue: cmd/compile: split codegen tests into one file for each arch

@ALTree

This comment has been minimized.

Copy link
Member Author

commented Mar 5, 2018

Tests are now being moved to the new codegen test harness; closing here.

@ALTree ALTree closed this Mar 5, 2018

@golang golang locked and limited conversation to collaborators Mar 5, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.