Re: [PATCH v3 3/4] tests: Do not occlude grub-shell return code

[Daniel Kiper]

On Thu, Oct 07, 2021 at 03:33:26PM -0500, Glenn Washburn wrote:
> The script grub-shell does the bulk of the testing. If it returns an error
> code, that means that the test failed and the test should immediately exit
> with that error code. When grub-shell is used as a non-terminating command
> in a pipeline, eg. when data needs to be extracted from its output, its
> error code will be occluded by the last command in the pipeline. Refactor
> tests so that the shell will error with the exit code of grub-shell by
> breaking up pipelines such that grub-shell is always the last command in the
> pipeline that it is used in.
>
> Signed-off-by: Glenn Washburn <development@efficientek.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> except...

> ---
>  tests/ahci_test.in             | 4 ++--
>  tests/ehci_test.in             | 4 ++--
>  tests/grub_cmd_date.in         | 3 ++-
>  tests/grub_script_expansion.in | 3 ++-
>  tests/ohci_test.in             | 4 ++--
>  tests/partmap_test.in          | 4 ++--
>  tests/uhci_test.in             | 4 ++--
>  7 files changed, 14 insertions(+), 12 deletions(-)

[...]

> diff --git a/tests/grub_cmd_date.in b/tests/grub_cmd_date.in
> index f7c9ca004..f9156691e 100644
> --- a/tests/grub_cmd_date.in
> +++ b/tests/grub_cmd_date.in
> @@ -9,7 +9,8 @@ if [ "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" = 
> sparc64-ieee1275 ];
>  fi
>
>  pdt="$(date -u +%s)"
> -dt=`echo date | @builddir@/grub-shell | sed 's, [A-Z][a-z]*$,,'`
> +dt=`echo date | @builddir@/grub-shell`
> +dt=`echo "$dt" | sed 's, [A-Z][a-z]*$,,'`

If you change this line could you change "`...`" to "$(...)"? to align
this code with what is before and after it?

>  dtg="$(date -u -d "$dt" +%s)"
>  ndt="$(date -u +%s)"
>
> diff --git a/tests/grub_script_expansion.in b/tests/grub_script_expansion.in
> index 9d0dcdd29..98d5a9068 100644
> --- a/tests/grub_script_expansion.in
> +++ b/tests/grub_script_expansion.in
> @@ -17,7 +17,8 @@ set -e
>  # You should have received a copy of the GNU General Public License
>  # along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>
> -disks=`echo ls | @builddir@/grub-shell| grep -av '^Network protocols:$'| 
> grep -av '^tftp http $'`
> +disks=`echo ls | @builddir@/grub-shell`
> +disks=`echo "$disks"| grep -av '^Network protocols:$'| grep -av '^tftp http 
> $'`
>  other=`echo insmod regexp\; echo \* | @builddir@/grub-shell`

Ugh... Next "`...`" usage... Though I would live it as is to not complicate
this patch... Or fix this in separate patch if you do not mind...

Daniel