diff mbox series

test_env: don't strip() printenv results

Message ID 20191218183721.107623-1-swarren@wwwdotorg.org
State Accepted
Commit fc1a3bfedf7d6cca27a8460b419482b8b0959e27
Delegated to: Tom Rini
Headers show
Series test_env: don't strip() printenv results | expand

Commit Message

Stephen Warren Dec. 18, 2019, 6:37 p.m. UTC 
From: Stephen Warren <swarren@nvidia.com>

get_env() was originally written to strip() the output of printenv to
isolate the test from any whitespace changes in printenv's output.
However, this throws away any whitespace in the variable value, which can
cause issues when test code expects to see that whitespace. In fact,
printenv never adds any whitespace at all, so there's no need to strip.

The strip causes a practical problem for test_env_echo_exists() if
state_test_env.get_existent_var() happens to choose a U-Boot variable that
contains trailing whitespace. This is true for variable boot_targets.

With Python 2, get_existent_var() never returned boot_targets so this
issue never caused a practical problem.

With Python 3, get_existent_var does sometimes return boot_targets, no
doubt due to Python 3's different dict hash key order implementation,
about 0.5-2% of the time, so this test appears intermittent. With the
strip removed, this intermittency is solved, since the test passes for all
possible U-Boot variables.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 test/py/tests/test_env.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tom Rini Jan. 2, 2020, 11:11 p.m. UTC | #1 
On Wed, Dec 18, 2019 at 11:37:21AM -0700, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> get_env() was originally written to strip() the output of printenv to
> isolate the test from any whitespace changes in printenv's output.
> However, this throws away any whitespace in the variable value, which can
> cause issues when test code expects to see that whitespace. In fact,
> printenv never adds any whitespace at all, so there's no need to strip.
> 
> The strip causes a practical problem for test_env_echo_exists() if
> state_test_env.get_existent_var() happens to choose a U-Boot variable that
> contains trailing whitespace. This is true for variable boot_targets.
> 
> With Python 2, get_existent_var() never returned boot_targets so this
> issue never caused a practical problem.
> 
> With Python 3, get_existent_var does sometimes return boot_targets, no
> doubt due to Python 3's different dict hash key order implementation,
> about 0.5-2% of the time, so this test appears intermittent. With the
> strip removed, this intermittency is solved, since the test passes for all
> possible U-Boot variables.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
index 9bdaef9373fd..6ff38f1020b5 100644
--- a/test/py/tests/test_env.py
+++ b/test/py/tests/test_env.py
@@ -49,7 +49,7 @@  class StateTestEnv(object):
         for l in response.splitlines():
             if not '=' in l:
                 continue
-            (var, value) = l.strip().split('=', 1)
+            (var, value) = l.split('=', 1)
             self.env[var] = value
 
     def get_existent_var(self):