On 14.08.23 16:32, 'Earl Chew' via EFI Boot Guard wrote:
> Jan,
> 
>> Just augment the line that installs bats & Co.
> 
> All right.
> 
>> For which new statement do we need that dep? I'm not spotting it yet.
> 
> This is required to provide /usr/bin/crc32.
> 
>>> +          time bats --tap ../tests
>>>             popd >/dev/null
>>> -          time bats --tap tests
>>
>> Why that?
> 
> The configuration artifacts are located in build/ which was created
> as part of the github workflow, but user workflows are free to use
> whatever directory they like or even build in the source tree.
> 
> See my reply below about configure.ac.
> 
> This change assumes that running the BATS test from the directory
> containing the build artifacts is a reasonable thing to do.
> 
>>> +    grep -qF "ENV_MEM_USERVARS=$targetsize"
> "$BATS_TEST_DIRNAME"/../configure.ac || return $?
>>
>> You probably wanted to check config.h or config.log. The above will
>> always bail out because the default is your targetsize. But that will
>> mean having the path to the build dir still handy.
> 
> The check against configure.ac is to ensure that the MD5 checksum
> computed against ENV_MEM_USERVARS=131072 is plausibly valid.

Ok, now I see. But then why not use the defaultsize from configure.ac as
targetsize?

> 
> configure.ac:           ENV_MEM_USERVARS=131072
> 
> Maybe renaming $targetsize to $defaultsize would be helpful.
> 
> The check against config.h that follows this is to determine the size
> selected by the user at configuration time. This determines the actual
> present size of the uservars array:
> 
>  +    local envsize
>  +    envsize=$(awk '/ ENV_MEM_USERVARS / { print $3 }' config.h)
> 
>> How about checking the target file size against the $targetsize?
> 
> Given that the size intended by the user is extracted from config.h,
> I think using that to confirm the present size would be preferred, since
> the size should match the configured value.
> 
> Including such a test would make the BATS test a bit more brittle
> since the file also includes other parts of the BG_ENVDATA structure.
> 
> Do you think that's acceptable?

Makes sense now. I'm fine with extracting from config.h, but please
provide a hint if that file is not found, rather than likely exploding.
So far, it was fine to run the tests outside of the build folder, and
people may be surprised if things silently fail now.

Jan

-- 
Siemens AG, Technology
Linux Expert Center

-- 
You received this message because you are subscribed to the Google Groups "EFI 
Boot Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/efibootguard-dev/4c7b89ec-19c2-45da-95f5-9d22965dfd98%40siemens.com.

Reply via email to