-
Notifications
You must be signed in to change notification settings - Fork 44
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
Fixing release CI failures and adding frozen 1.1 ROM #1809
base: main
Are you sure you want to change the base?
Conversation
@@ -420,11 +420,6 @@ jobs: | |||
TEST_BIN=/tmp/caliptra-test-binaries | |||
VARS="CPTRA_UIO_NUM=4 CALIPTRA_PREBUILT_FW_DIR=/tmp/caliptra-test-firmware CALIPTRA_IMAGE_NO_GIT_REVISION=1" | |||
if [[ "${{ inputs.workflow_call }}" && "${{ inputs.hw-version }}" != "latest" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now automatically chosen in the FIPS testing framework when we use the CPTRA_CI_ROM_VERSION environment variable
runtime/Cargo.toml
Outdated
@@ -65,3 +65,5 @@ no-cfi = ["caliptra-image-verify/no-cfi", "caliptra-drivers/no-cfi"] | |||
fpga_realtime = ["caliptra-drivers/fpga_realtime"] | |||
"hw-1.0" = ["caliptra-builder/hw-1.0", "caliptra-drivers/hw-1.0", "caliptra-registers/hw-1.0", "caliptra-kat/hw-1.0","caliptra-cpu/hw-1.0"] | |||
fips-test-hooks = ["caliptra-drivers/fips-test-hooks"] | |||
"ci-rom-1.0" = ["caliptra-builder/ci-rom-1.0"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm nervous adding rom-version-specific features to the entire crate, as it will be tempting for the runtime firmware to look at these and change it's behavior.
Alternatives:
- use environment variables to tell the test which logic to execute (my preferred)
- Move the tests to a separate crate that only has tests, and add this feature there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this a bit more, maybe this is not a problem as long as caliptra-builder NEVER passes this feature when it builds the firmware (if we keep the feature we should confirm that).
That said, the existing rom_from_env() function uses an environment variable to decide between UART and non-UART, maybe it could do something similar for the ROM version.... You could add a similar function that returns the expected ROM version that test cases query to change their behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change has been made. Instead of features, it uses the environment variable "CPTRA_CI_ROM_VERSION". get_ci_rom_version() provides the version to both the tests and the function that provides the ROM binary (rom_for_fw_integration_tests).
2727939
to
b278314
Compare
b278314
to
5f87e7f
Compare
Prevents a release being made from main when tests were actuall run on a different branch/commit
This run shows it solving all the test failures in the nightly. (The final publish step fails as expected since this is not the main branch). |
No description provided.