-
Notifications
You must be signed in to change notification settings - Fork 9
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
Mozak-loader tests based on empty-elf #1049
Conversation
Tiny PR, to help with mozak-loader testing (part of #1049) Co-authored-by: Matthias Görgens <matthias.goergens@mozak.com>
fn test_empty_elf_check_assumed_values() { | ||
// This test ensures mozak-loader & mozak-linker-script is indeed aligned | ||
let mozak_ro_memory = Program::mozak_load_program( | ||
mozak_examples::EMPTY_ELF, |
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.
Does this test only work for EMPTY_ELF? I think it should work for any ELF
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 assuming it'd be more tedious to check every ELF that we have, since they might not all share (for eg.) similar run time args?
My understanding of this PR is that this ensures our loader is working as expected, and isn't necessarily a test for ELFs specifically, so an empty one would serve that purpose - we could have a followup PR if necessary?
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.
1 small comment/question
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.
LGTM, deferring to @vivekvpandya regarding the ELF comment/conversation
examples/empty
ELFrunner/src/elf.rs
that use this empty-elf to checkmozak-loader