diff mbox series

[U-Boot,8/8] test/py: vboot: Remove stderr redirect from openssl command

Message ID 20170914200634.17818-9-paul.burton@imgtec.com
State Superseded
Delegated to: Tom Rini
Headers show
Series test/py: Fixes for python 3.x | expand

Commit Message

Paul Burton Sept. 14, 2017, 8:06 p.m. UTC
The openssl command specified in test_with_algo() ultimately ends up
being run by RunAndLog::run(), which uses it to construct a Popen object
with the default shell=False. The stderr redirect in the command is
therefore simply passed to openssl as an argument. With at least openssl
1.1.0f this causes openssl, and therefore test_vboot, to fail with:

  genpkey: Use -help for summary.
  Exit code: 1

Any stderr output ought to be captured & stored in the RunAndLog
object's output field and returned from run() via run_and_log() to
test_with_algo() which then ignores it anyway, so we can drop the
shell-like redirection with no ill effects. With this fix test_vboot now
passes for me.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>

---

 test/py/tests/test_vboot.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Stephen Warren Sept. 14, 2017, 8:28 p.m. UTC | #1
On 09/14/2017 02:06 PM, Paul Burton wrote:
> The openssl command specified in test_with_algo() ultimately ends up
> being run by RunAndLog::run(), which uses it to construct a Popen object
> with the default shell=False. The stderr redirect in the command is
> therefore simply passed to openssl as an argument. With at least openssl
> 1.1.0f this causes openssl, and therefore test_vboot, to fail with:
> 
>    genpkey: Use -help for summary.
>    Exit code: 1
> 
> Any stderr output ought to be captured & stored in the RunAndLog
> object's output field and returned from run() via run_and_log() to
> test_with_algo() which then ignores it anyway, so we can drop the
> shell-like redirection with no ill effects. With this fix test_vboot now
> passes for me.

> diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py

>       util.run_and_log(cons, 'openssl genpkey -algorithm RSA -out %sdev.key '
>                        '-pkeyopt rsa_keygen_bits:2048 '
> -                     '-pkeyopt rsa_keygen_pubexp:%d '
> -                     '2>/dev/null'  % (tmpdir, public_exponent))
> +                     '-pkeyopt rsa_keygen_pubexp:%d'
> +                     % (tmpdir, public_exponent))

Nit: Operators are usually placed at the end of a line rather than at 
the start of the next:

 > +                     '-pkeyopt rsa_keygen_pubexp:%d' %
 > +                     (tmpdir, public_exponent))

... but again not a big deal.

I could have sworn I checked this code ran on Python 3 when I first 
wrote it. Some of the issues you've fixed were clearly introduced since 
then, but not all. Perhaps I'm remembering some other project where I 
used test.py instead. Anyway, the series,

Reviewed-by: Stephen Warren <swarren@nvidia.com>
Paul Burton Sept. 14, 2017, 9:37 p.m. UTC | #2
Hi Stephen,

On Thursday, 14 September 2017 13:28:35 PDT Stephen Warren wrote:
> I could have sworn I checked this code ran on Python 3 when I first
> wrote it. Some of the issues you've fixed were clearly introduced since
> then, but not all. Perhaps I'm remembering some other project where I
> used test.py instead. Anyway, the series,
> 
> Reviewed-by: Stephen Warren <swarren@nvidia.com>

Thanks for the speedy review. I've addressed the nits you raised & submitted 
v2 with your Reviewed-by.

Thanks,
    Paul
diff mbox series

Patch

diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
index 6e62820743..ba86a08e2e 100644
--- a/test/py/tests/test_vboot.py
+++ b/test/py/tests/test_vboot.py
@@ -174,8 +174,8 @@  def test_vboot(u_boot_console):
     public_exponent = 65537
     util.run_and_log(cons, 'openssl genpkey -algorithm RSA -out %sdev.key '
                      '-pkeyopt rsa_keygen_bits:2048 '
-                     '-pkeyopt rsa_keygen_pubexp:%d '
-                     '2>/dev/null'  % (tmpdir, public_exponent))
+                     '-pkeyopt rsa_keygen_pubexp:%d'
+                     % (tmpdir, public_exponent))
 
     # Create a certificate containing the public key
     util.run_and_log(cons, 'openssl req -batch -new -x509 -key %sdev.key -out '