Refactor efi/preinstall#552
Merged
Merged
Conversation
valentindavid
approved these changes
Jun 22, 2026
There was a problem hiding this comment.
Pull request overview
This PR refactors parts of efi/preinstall to reduce test duplication and simplify error/action handling in RunChecksContext, while also adding top-level project metadata (license/readme/make targets) to improve repo usability.
Changes:
- Extracts error classification/action derivation into
RunChecksContext.convertErrorsToActionsand simplifiesRun()control flow. - Deduplicates a large amount of preinstall test setup by introducing shared mock MEI
fw_statusconstants and a shared default EFI image set. - Adds
README.md,COPYING, and aMakefilewith convenient build/test/coverage targets; increasesrun-teststimeout.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| run-tests | Extends go test timeout to 30m. |
| README.md | Adds a brief project overview, features list, and license pointer. |
| Makefile | Adds build + test convenience targets, including focused efi/preinstall coverage runs. |
| internal/efitest/wincert.go | Splits Authenticode detached win-cert creation into a reusable base helper and adds “must” variant. |
| efi/preinstall/preinstall_test.go | Precomputes a reusable WinCertificate instance for tests. |
| efi/preinstall/intel_mei_test.go | Introduces shared mock fw_status register encodings for MEI/BootGuard tests. |
| efi/preinstall/efi_images_test.go | Adds shared default mock EFI images + digests for tests. |
| efi/preinstall/checks_test.go | Switches many tests to use shared MEI fw_status data and shared default images. |
| efi/preinstall/checks_context.go | Refactors RunChecksContext.Run() by extracting error→actions conversion logic. |
| efi/preinstall/check_pcr7_test.go | Uses shared digest constants to reduce repetition. |
| efi/preinstall/check_pcr4_test.go | Uses shared digest constants to reduce repetition. |
| efi/preinstall/check_host_security_intel_test.go | Uses shared MEI fw_status test vectors. |
| efi/preinstall/check_host_security_amd64_test.go | Uses shared MEI fw_status test vectors. |
| COPYING | Adds GPLv3 license text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pedronis
reviewed
Jun 22, 2026
pedronis
left a comment
Collaborator
There was a problem hiding this comment.
@frederic-hoerni thanks for splitting the refactorings, some of the copilot comments seemsat least worth answering to
Collaborator
Author
|
I took into account the remarks. I will squash the relevant commits when all will be approved. |
Duplicate hard-coded values are put in a common place.
This function enables reading a win certificate out of a test context.
This is needed only once at the beginning of a test campaign and there is no point in loading these again for each test.
frederic-hoerni
force-pushed
the
refactor-preinstall
branch
from
June 22, 2026 14:58
814437f to
714d9bd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This modification refactors part of efi/preinstall to reduce duplicated code in tests and split Run() into smaller functions.
It also adds license, readme and makefile to give a little more context about secboot.