From: Stephen Warren swarren@nvidia.com

After a test has failed, test/py drains the U-Boot console log to ensure that any relevant output is captured. At this point, we don't care about detecting any additional errors, since the test is already known to have failed, and U-Boot will be restarted. To ensure that the test cleanup code is not interrupted, and can correctly terminate the log sections for the failed test, ignore any exception that occurs while reading the U-Boot console output during this limited period of time.

Signed-off-by: Stephen Warren swarren@nvidia.com Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: None

test/py/u_boot_console_base.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index a14bad6e8c5..326b2ac51fb 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -304,7 +304,17 @@ class ConsoleBase(object): # Wait for something U-Boot will likely never send. This will # cause the console output to be read and logged. self.p.expect(['This should never match U-Boot output']) - except u_boot_spawn.Timeout: + except: + # We expect a timeout, since U-Boot won't print what we waited + # for. Squash it when it happens. + # + # Squash any other exception too. This function is only used to + # drain (and log) the U-Boot console output after a failed test. + # The U-Boot process will be restarted, or target board reset, once + # this function returns. So, we don't care about detecting any + # additional errors, so they're squashed so that the rest of the + # post-test-failure cleanup code can continue operation, and + # correctly terminate any log sections, etc. pass finally: self.p.timeout = orig_timeout

At present the unicode filtering seems to get confused at times with this error:

UnicodeDecodeError: 'ascii' codec can't decode byte 0x80 in position 32: ordinal not in range(128)

It seems to be due to self._nonprint being interpreted as UTF-8. Fix it by using ordinals instead of characters, changing the string to set.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Stephen Warren swarren@nvidia.com ---

Changes in v2: None

test/py/multiplexed_log.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/test/py/multiplexed_log.py b/test/py/multiplexed_log.py index f23d5dec68c..637a3bd257b 100644 --- a/test/py/multiplexed_log.py +++ b/test/py/multiplexed_log.py @@ -314,8 +314,9 @@ $(document).ready(function () {

# The set of characters that should be represented as hexadecimal codes in # the log file. - _nonprint = ('%' + ''.join(chr(c) for c in range(0, 32) if c not in (9, 10)) + - ''.join(chr(c) for c in range(127, 256))) + _nonprint = {ord('%')} + _nonprint.update({c for c in range(0, 32) if c not in (9, 10)}) + _nonprint.update({c for c in range(127, 256)})

def _escape(self, data): """Render data format suitable for inclusion in an HTML document. @@ -331,7 +332,7 @@ $(document).ready(function () { """

data = data.replace(chr(13), '') - data = ''.join((c in self._nonprint) and ('%%%02x' % ord(c)) or + data = ''.join((ord(c) in self._nonprint) and ('%%%02x' % ord(c)) or c for c in data) data = cgi.escape(data) return data

At present some helper functions are mixed in with the tests. Tidy this up by moving them to the top. For the few helpers that don't need to be full class members, make them nested functions.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: None

tools/binman/ftest.py | 249 +++++++++++++++++++++--------------------- 1 file changed, 124 insertions(+), 125 deletions(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index abf02b62e81..b90928e738a 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -421,6 +421,111 @@ class TestFunctional(unittest.TestCase): AddNode(dtb.GetRoot(), '') return tree

+ def _RunMicrocodeTest(self, dts_fname, nodtb_data, ucode_second=False): + """Handle running a test for insertion of microcode + + Args: + dts_fname: Name of test .dts file + nodtb_data: Data that we expect in the first section + ucode_second: True if the microsecond entry is second instead of + third + + Returns: + Tuple: + Contents of first region (U-Boot or SPL) + Offset and size components of microcode pointer, as inserted + in the above (two 4-byte words) + """ + data = self._DoReadFile(dts_fname, True) + + # Now check the device tree has no microcode + if ucode_second: + ucode_content = data[len(nodtb_data):] + ucode_pos = len(nodtb_data) + dtb_with_ucode = ucode_content[16:] + fdt_len = self.GetFdtLen(dtb_with_ucode) + else: + dtb_with_ucode = data[len(nodtb_data):] + fdt_len = self.GetFdtLen(dtb_with_ucode) + ucode_content = dtb_with_ucode[fdt_len:] + ucode_pos = len(nodtb_data) + fdt_len + fname = tools.GetOutputFilename('test.dtb') + with open(fname, 'wb') as fd: + fd.write(dtb_with_ucode) + dtb = fdt.FdtScan(fname) + ucode = dtb.GetNode('/microcode') + self.assertTrue(ucode) + for node in ucode.subnodes: + self.assertFalse(node.props.get('data')) + + # Check that the microcode appears immediately after the Fdt + # This matches the concatenation of the data properties in + # the /microcode/update@xxx nodes in 34_x86_ucode.dts. + ucode_data = struct.pack('>4L', 0x12345678, 0x12345679, 0xabcd0000, + 0x78235609) + self.assertEqual(ucode_data, ucode_content[:len(ucode_data)]) + + # Check that the microcode pointer was inserted. It should match the + # expected offset and size + pos_and_size = struct.pack('<2L', 0xfffffe00 + ucode_pos, + len(ucode_data)) + u_boot = data[:len(nodtb_data)] + return u_boot, pos_and_size + + def _RunPackUbootSingleMicrocode(self): + """Test that x86 microcode can be handled correctly + + We expect to see the following in the image, in order: + u-boot-nodtb.bin with a microcode pointer inserted at the correct + place + u-boot.dtb with the microcode + an empty microcode region + """ + # We need the libfdt library to run this test since only that allows + # finding the offset of a property. This is required by + # Entry_u_boot_dtb_with_ucode.ObtainContents(). + data = self._DoReadFile('35_x86_single_ucode.dts', True) + + second = data[len(U_BOOT_NODTB_DATA):] + + fdt_len = self.GetFdtLen(second) + third = second[fdt_len:] + second = second[:fdt_len] + + ucode_data = struct.pack('>2L', 0x12345678, 0x12345679) + self.assertIn(ucode_data, second) + ucode_pos = second.find(ucode_data) + len(U_BOOT_NODTB_DATA) + + # Check that the microcode pointer was inserted. It should match the + # expected offset and size + pos_and_size = struct.pack('<2L', 0xfffffe00 + ucode_pos, + len(ucode_data)) + first = data[:len(U_BOOT_NODTB_DATA)] + self.assertEqual('nodtb with microcode' + pos_and_size + + ' somewhere in here', first) + + def _PackUbootSplMicrocode(self, dts, ucode_second=False): + """Helper function for microcode tests + + We expect to see the following in the image, in order: + u-boot-spl-nodtb.bin with a microcode pointer inserted at the + correct place + u-boot.dtb with the microcode removed + the microcode + + Args: + dts: Device tree file to use for test + ucode_second: True if the microsecond entry is second instead of + third + """ + # ELF file with a '_dt_ucode_base_size' symbol + with open(self.TestFile('u_boot_ucode_ptr')) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + first, pos_and_size = self._RunMicrocodeTest(dts, U_BOOT_SPL_NODTB_DATA, + ucode_second=ucode_second) + self.assertEqual('splnodtb with microc' + pos_and_size + + 'ter somewhere in here', first) + def testRun(self): """Test a basic run with valid args""" result = self._RunBinman('-h') @@ -808,57 +913,6 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('33_x86-start16.dts') self.assertEqual(X86_START16_DATA, data[:len(X86_START16_DATA)])

- def _RunMicrocodeTest(self, dts_fname, nodtb_data, ucode_second=False): - """Handle running a test for insertion of microcode - - Args: - dts_fname: Name of test .dts file - nodtb_data: Data that we expect in the first section - ucode_second: True if the microsecond entry is second instead of - third - - Returns: - Tuple: - Contents of first region (U-Boot or SPL) - Offset and size components of microcode pointer, as inserted - in the above (two 4-byte words) - """ - data = self._DoReadFile(dts_fname, True) - - # Now check the device tree has no microcode - if ucode_second: - ucode_content = data[len(nodtb_data):] - ucode_pos = len(nodtb_data) - dtb_with_ucode = ucode_content[16:] - fdt_len = self.GetFdtLen(dtb_with_ucode) - else: - dtb_with_ucode = data[len(nodtb_data):] - fdt_len = self.GetFdtLen(dtb_with_ucode) - ucode_content = dtb_with_ucode[fdt_len:] - ucode_pos = len(nodtb_data) + fdt_len - fname = tools.GetOutputFilename('test.dtb') - with open(fname, 'wb') as fd: - fd.write(dtb_with_ucode) - dtb = fdt.FdtScan(fname) - ucode = dtb.GetNode('/microcode') - self.assertTrue(ucode) - for node in ucode.subnodes: - self.assertFalse(node.props.get('data')) - - # Check that the microcode appears immediately after the Fdt - # This matches the concatenation of the data properties in - # the /microcode/update@xxx nodes in 34_x86_ucode.dts. - ucode_data = struct.pack('>4L', 0x12345678, 0x12345679, 0xabcd0000, - 0x78235609) - self.assertEqual(ucode_data, ucode_content[:len(ucode_data)]) - - # Check that the microcode pointer was inserted. It should match the - # expected offset and size - pos_and_size = struct.pack('<2L', 0xfffffe00 + ucode_pos, - len(ucode_data)) - u_boot = data[:len(nodtb_data)] - return u_boot, pos_and_size - def testPackUbootMicrocode(self): """Test that x86 microcode can be handled correctly

@@ -873,38 +927,6 @@ class TestFunctional(unittest.TestCase): self.assertEqual('nodtb with microcode' + pos_and_size + ' somewhere in here', first)

- def _RunPackUbootSingleMicrocode(self): - """Test that x86 microcode can be handled correctly - - We expect to see the following in the image, in order: - u-boot-nodtb.bin with a microcode pointer inserted at the correct - place - u-boot.dtb with the microcode - an empty microcode region - """ - # We need the libfdt library to run this test since only that allows - # finding the offset of a property. This is required by - # Entry_u_boot_dtb_with_ucode.ObtainContents(). - data = self._DoReadFile('35_x86_single_ucode.dts', True) - - second = data[len(U_BOOT_NODTB_DATA):] - - fdt_len = self.GetFdtLen(second) - third = second[fdt_len:] - second = second[:fdt_len] - - ucode_data = struct.pack('>2L', 0x12345678, 0x12345679) - self.assertIn(ucode_data, second) - ucode_pos = second.find(ucode_data) + len(U_BOOT_NODTB_DATA) - - # Check that the microcode pointer was inserted. It should match the - # expected offset and size - pos_and_size = struct.pack('<2L', 0xfffffe00 + ucode_pos, - len(ucode_data)) - first = data[:len(U_BOOT_NODTB_DATA)] - self.assertEqual('nodtb with microcode' + pos_and_size + - ' somewhere in here', first) - def testPackUbootSingleMicrocode(self): """Test that x86 microcode can be handled correctly with fdt_normal. """ @@ -1020,28 +1042,6 @@ class TestFunctional(unittest.TestCase): data = self._DoReadFile('48_x86-start16-spl.dts') self.assertEqual(X86_START16_SPL_DATA, data[:len(X86_START16_SPL_DATA)])

- def _PackUbootSplMicrocode(self, dts, ucode_second=False): - """Helper function for microcode tests - - We expect to see the following in the image, in order: - u-boot-spl-nodtb.bin with a microcode pointer inserted at the - correct place - u-boot.dtb with the microcode removed - the microcode - - Args: - dts: Device tree file to use for test - ucode_second: True if the microsecond entry is second instead of - third - """ - # ELF file with a '_dt_ucode_base_size' symbol - with open(self.TestFile('u_boot_ucode_ptr')) as fd: - TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) - first, pos_and_size = self._RunMicrocodeTest(dts, U_BOOT_SPL_NODTB_DATA, - ucode_second=ucode_second) - self.assertEqual('splnodtb with microc' + pos_and_size + - 'ter somewhere in here', first) - def testPackUbootSplMicrocode(self): """Test that x86 microcode can be handled correctly in SPL""" self._PackUbootSplMicrocode('49_x86_ucode_spl.dts') @@ -1324,19 +1324,18 @@ class TestFunctional(unittest.TestCase): self.assertIn("'fill' entry must have a size property", str(e.exception))

- def _HandleGbbCommand(self, pipe_list): - """Fake calls to the futility utility""" - if pipe_list[0][0] == 'futility': - fname = pipe_list[0][-1] - # Append our GBB data to the file, which will happen every time the - # futility command is called. - with open(fname, 'a') as fd: - fd.write(GBB_DATA) - return command.CommandResult() - def testGbb(self): """Test for the Chromium OS Google Binary Block""" - command.test_result = self._HandleGbbCommand + def _HandleGbbCommand(pipe_list): + """Fake calls to the futility utility""" + if pipe_list[0][0] == 'futility': + fname = pipe_list[0][-1] + # Append our GBB data to the file, which will happen every time + # the futility command is called. + with open(fname, 'a') as fd: + fd.write(GBB_DATA) + return command.CommandResult() + command.test_result = _HandleGbbCommand entry_args = { 'keydir': 'devkeys', 'bmpblk': 'bmpblk.bin', @@ -1361,17 +1360,17 @@ class TestFunctional(unittest.TestCase): self.assertIn("Node '/binman/gbb': GBB must have a fixed size", str(e.exception))

- def _HandleVblockCommand(self, pipe_list): - """Fake calls to the futility utility""" - if pipe_list[0][0] == 'futility': - fname = pipe_list[0][3] - with open(fname, 'wb') as fd: - fd.write(VBLOCK_DATA) - return command.CommandResult() - def testVblock(self): """Test for the Chromium OS Verified Boot Block""" - command.test_result = self._HandleVblockCommand + def _HandleVblockCommand(pipe_list): + """Fake calls to the futility utility""" + if pipe_list[0][0] == 'futility': + fname = pipe_list[0][3] + with open(fname, 'wb') as fd: + fd.write(VBLOCK_DATA) + return command.CommandResult() + + command.test_result = _HandleVblockCommand entry_args = { 'keydir': 'devkeys', }

Use 'export' to avoid repeating the path setup for each command.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: None

test/run | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/test/run b/test/run index d77a1c371b4..6b73813f9bc 100755 --- a/test/run +++ b/test/run @@ -16,23 +16,23 @@ run_test ./test/py/test.py --bd sandbox_spl --build -k test_ofplatdata.py # Run tests for the flat DT version of sandbox ./test/py/test.py --bd sandbox_flattree --build

+# Set up a path to dtc (device-tree compiler) and libfdt.py, a library it +# provides and which is built by the sandbox_spl config. DTC_DIR=build-sandbox_spl/scripts/dtc +export PYTHONPATH=${DTC_DIR}/pylibfdt +export DTC=${DTC_DIR}/dtc

-PYTHONPATH=${DTC_DIR}/pylibfdt DTC=${DTC_DIR}/dtc run_test \ - ./tools/binman/binman -t +run_test ./tools/binman/binman -t run_test ./tools/patman/patman --test run_test ./tools/buildman/buildman -t -PYTHONPATH=${DTC_DIR}/pylibfdt DTC=${DTC_DIR}/dtc run_test ./tools/dtoc/dtoc -t +run_test ./tools/dtoc/dtoc -t

# This needs you to set up Python test coverage tools. # To enable Python test coverage on Debian-type distributions (e.g. Ubuntu): # $ sudo apt-get install python-pytest python-coverage -PYTHONPATH=${DTC_DIR}/pylibfdt DTC=${DTC_DIR}/dtc run_test \ - ./tools/binman/binman -T -PYTHONPATH=${DTC_DIR}/pylibfdt DTC=${DTC_DIR}/dtc run_test \ - ./tools/dtoc/dtoc -T -PYTHONPATH=${DTC_DIR}/pylibfdt DTC=${DTC_DIR}/dtc run_test \ - ./tools/dtoc/test_fdt -T +run_test ./tools/binman/binman -T +run_test ./tools/dtoc/dtoc -T +run_test ./tools/dtoc/test_fdt -T

if [ $result == 0 ]; then echo "Tests passed!"

The 'result' variable counts the number of failures in running the tests. Rename it to 'failures' to make this more obvious. Also tidy up a few comments.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: None

test/run | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/test/run b/test/run index d9901ae9f72..d64581ab140 100755 --- a/test/run +++ b/test/run @@ -9,10 +9,10 @@ run_test() { echo -n "$1: " shift "$@" - [ $? -ne 0 ] && result=$((result+1)) + [ $? -ne 0 ] && failures=$((failures+1)) }

-result=0 +failures=0

# Run all tests that the standard sandbox build can support run_test "sandbox" ./test/py/test.py --bd sandbox --build @@ -21,7 +21,10 @@ run_test "sandbox" ./test/py/test.py --bd sandbox --build run_test "sandbox_spl" ./test/py/test.py --bd sandbox_spl --build \ -k test_ofplatdata.py

-# Run tests for the flat DT version of sandbox +# Run tests for the flat-device-tree version of sandbox. This is a special +# build which does not enable CONFIG_OF_LIVE for the live device tree, so we can +# check that functionality is the same. The standard sandbox build (above) uses +# CONFIG_OF_LIVE. run_test "sandbox_flattree" ./test/py/test.py --bd sandbox_flattree --build

# Set up a path to dtc (device-tree compiler) and libfdt.py, a library it @@ -33,6 +36,7 @@ export DTC=${DTC_DIR}/dtc run_test "binman" ./tools/binman/binman -t run_test "patman" ./tools/patman/patman --test run_test "buildman" ./tools/buildman/buildman -t +run_test "fdt" ./tools/dtoc/test_fdt -t run_test "dtoc" ./tools/dtoc/dtoc -t

# This needs you to set up Python test coverage tools. @@ -42,7 +46,7 @@ run_test "binman code coverage" ./tools/binman/binman -T run_test "dtoc code coverage" ./tools/dtoc/dtoc -T run_test "fdt code coverage" ./tools/dtoc/test_fdt -T

-if [ $result == 0 ]; then +if [ $failures == 0 ]; then echo "Tests passed!" else echo "Tests FAILED"

At present 'make check' leaves some temporary directories around. Part of this is because we call tools.PrepareOutputDir() twice in some cases, without calling tools.FinaliseOutputDir() in between.

Fix this.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: None

tools/binman/elf_test.py | 5 +++++ tools/binman/entry_test.py | 8 ++++++-- tools/binman/fdt_test.py | 4 ++++ tools/binman/ftest.py | 8 +++----- tools/dtoc/test_fdt.py | 10 +++++++--- 5 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index c16f71401d1..b68530c19ba 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -10,6 +10,7 @@ import unittest

import elf import test_util +import tools

binman_dir = os.path.dirname(os.path.realpath(sys.argv[0]))

@@ -46,6 +47,10 @@ class FakeSection:

class TestElf(unittest.TestCase): + @classmethod + def setUpClass(self): + tools.SetInputDirs(['.']) + def testAllSymbols(self): """Test that we can obtain a symbol from the ELF file""" fname = os.path.join(binman_dir, 'test', 'u_boot_ucode_ptr') diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py index 69d85b4cedb..a8bc938f9e9 100644 --- a/tools/binman/entry_test.py +++ b/tools/binman/entry_test.py @@ -14,9 +14,14 @@ import fdt_util import tools

class TestEntry(unittest.TestCase): + def setUp(self): + tools.PrepareOutputDir(None) + + def tearDown(self): + tools.FinaliseOutputDir() + def GetNode(self): binman_dir = os.path.dirname(os.path.realpath(sys.argv[0])) - tools.PrepareOutputDir(None) fname = fdt_util.EnsureCompiled( os.path.join(binman_dir,('test/05_simple.dts'))) dtb = fdt.FdtScan(fname) @@ -35,7 +40,6 @@ class TestEntry(unittest.TestCase): global entry reload(entry) entry.Entry.Create(None, self.GetNode(), 'u-boot-spl') - tools._RemoveOutputDir() del entry

def testEntryContents(self): diff --git a/tools/binman/fdt_test.py b/tools/binman/fdt_test.py index 8ea098f38ae..b9167012d25 100644 --- a/tools/binman/fdt_test.py +++ b/tools/binman/fdt_test.py @@ -21,6 +21,10 @@ class TestFdt(unittest.TestCase): self._indir = tempfile.mkdtemp(prefix='binmant.') tools.PrepareOutputDir(self._indir, True)

+ @classmethod + def tearDownClass(self): + tools._FinaliseForTest() + def TestFile(self, fname): return os.path.join(self._binman_dir, 'test', fname)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index b90928e738a..a58804c10fe 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -113,7 +113,6 @@ class TestFunctional(unittest.TestCase): TestFunctional._MakeInputFile('ecrw.bin', CROS_EC_RW_DATA) TestFunctional._MakeInputDir('devkeys') TestFunctional._MakeInputFile('bmpblk.bin', BMPBLK_DATA) - self._output_setup = False

# ELF file with a '_dt_ucode_base_size' symbol with open(self.TestFile('u_boot_ucode_ptr')) as fd: @@ -228,14 +227,13 @@ class TestFunctional(unittest.TestCase): Returns: Contents of device-tree binary """ - if not self._output_setup: - tools.PrepareOutputDir(self._indir, True) - self._output_setup = True + tools.PrepareOutputDir(None) dtb = fdt_util.EnsureCompiled(self.TestFile(fname)) with open(dtb) as fd: data = fd.read() TestFunctional._MakeInputFile(outfile, data) - return data + tools.FinaliseOutputDir() + return data

def _GetDtbContentsForSplTpl(self, dtb_data, name): """Create a version of the main DTB for SPL or SPL diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index d2597020500..2e6febe8f38 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -60,7 +60,7 @@ class TestFdt(unittest.TestCase):

@classmethod def tearDownClass(cls): - tools._FinaliseForTest() + tools.FinaliseOutputDir()

def setUp(self): self.dtb = fdt.FdtScan('tools/dtoc/dtoc_test_simple.dts') @@ -128,7 +128,7 @@ class TestNode(unittest.TestCase):

@classmethod def tearDownClass(cls): - tools._FinaliseForTest() + tools.FinaliseOutputDir()

def setUp(self): self.dtb = fdt.FdtScan('tools/dtoc/dtoc_test_simple.dts') @@ -209,7 +209,7 @@ class TestProp(unittest.TestCase):

@classmethod def tearDownClass(cls): - tools._FinaliseForTest() + tools.FinaliseOutputDir()

def setUp(self): self.dtb = fdt.FdtScan('tools/dtoc/dtoc_test_simple.dts') @@ -427,6 +427,10 @@ class TestFdtUtil(unittest.TestCase): def setUpClass(cls): tools.PrepareOutputDir(None)

+ @classmethod + def tearDownClass(cls): + tools.FinaliseOutputDir() + def setUp(self): self.dtb = fdt.FdtScan('tools/dtoc/dtoc_test_simple.dts') self.node = self.dtb.GetNode('/spl-test')

There are a few test cases which print output. Suppress this so that tests can run silently in the normal case.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: None

tools/buildman/test.py | 4 +++- tools/dtoc/test_dtoc.py | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 7259f7b6507..e0c9d6da6a0 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -20,6 +20,7 @@ import control import command import commit import terminal +import test_util import toolchain

use_network = True @@ -422,7 +423,8 @@ class TestBuild(unittest.TestCase): def testToolchainDownload(self): """Test that we can download toolchains""" if use_network: - url = self.toolchains.LocateArchUrl('arm') + with test_util.capture_sys_output() as (stdout, stderr): + url = self.toolchains.LocateArchUrl('arm') self.assertRegexpMatches(url, 'https://www.kernel.org/pub/tools/' 'crosstool/files/bin/x86_64/.*/' 'x86_64-gcc-.*-nolibc_arm-.*linux-gnueabi.tar.xz') diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py index 72bcb37244e..11bead12607 100644 --- a/tools/dtoc/test_dtoc.py +++ b/tools/dtoc/test_dtoc.py @@ -385,7 +385,8 @@ U_BOOT_DEVICE(phandle_source2) = {

def test_phandle_bad(self): """Test a node containing an invalid phandle fails""" - dtb_file = get_dtb_file('dtoc_test_phandle_bad.dts') + dtb_file = get_dtb_file('dtoc_test_phandle_bad.dts', + capture_stderr=True) output = tools.GetOutputFilename('output') with self.assertRaises(ValueError) as e: dtb_platdata.run_steps(['struct'], dtb_file, False, output) @@ -394,7 +395,8 @@ U_BOOT_DEVICE(phandle_source2) = {

def test_phandle_bad2(self): """Test a phandle target missing its #*-cells property""" - dtb_file = get_dtb_file('dtoc_test_phandle_bad2.dts') + dtb_file = get_dtb_file('dtoc_test_phandle_bad2.dts', + capture_stderr=True) output = tools.GetOutputFilename('output') with self.assertRaises(ValueError) as e: dtb_platdata.run_steps(['struct'], dtb_file, False, output)

At present calling Uninit() always called ClearProgress() which outputs a \r character as well as spaces to remove any progress information on the line. This can mess up the normal output of binman and other tools. Fix this by outputing this only when progress information has actually been previous written.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: None

tools/patman/tout.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/patman/tout.py b/tools/patman/tout.py index 4cd49e1c685..4957c7ae1df 100644 --- a/tools/patman/tout.py +++ b/tools/patman/tout.py @@ -15,6 +15,8 @@ NOTICE = 2 INFO = 3 DEBUG = 4

+in_progress = False + """ This class handles output of progress and other useful information to the user. It provides for simple verbosity level control and can @@ -48,9 +50,11 @@ def UserIsPresent():

def ClearProgress(): """Clear any active progress message on the terminal.""" - if verbose > 0 and stdout_is_tty: + global in_progress + if verbose > 0 and stdout_is_tty and in_progress: _stdout.write('\r%s\r' % (" " * len (_progress))) _stdout.flush() + in_progress = False

def Progress(msg, warning=False, trailer='...'): """Display progress information. @@ -58,6 +62,7 @@ def Progress(msg, warning=False, trailer='...'): Args: msg: Message to display. warning: True if this is a warning.""" + global in_progress ClearProgress() if verbose > 0: _progress = msg + trailer @@ -65,6 +70,7 @@ def Progress(msg, warning=False, trailer='...'): col = _color.YELLOW if warning else _color.GREEN _stdout.write('\r' + _color.Color(col, _progress)) _stdout.flush() + in_progress = True else: _stdout.write(_progress + '\n')

At present the tests run one after the other using a single CPU. This is not very efficient. Bring in the concurrencytest module and run the tests concurrently, using one process for each CPU by default. A -P option allows this to be overridden, which is necessary for code-coverage to function correctly.

This requires fixing a few tests which are currently not fully independent.

At some point we might consider doing this across all pytests in U-Boot. There is a pytest version that supports specifying the number of processes to use, but it did not work for me.

Signed-off-by: Simon Glass sjg@chromium.org ---

Changes in v2: - Add a patch to run binman tests concurrently

.travis.yml | 1 + test/py/README.md | 1 + tools/binman/binman.py | 26 +++- tools/binman/cmdline.py | 2 + tools/binman/entry_test.py | 7 +- tools/binman/ftest.py | 34 +++--- tools/concurrencytest/.gitignore | 1 + tools/concurrencytest/README.md | 74 ++++++++++++ tools/concurrencytest/concurrencytest.py | 144 +++++++++++++++++++++++ tools/dtoc/dtoc.py | 2 + tools/dtoc/test_fdt.py | 2 + tools/patman/test_util.py | 2 +- 12 files changed, 274 insertions(+), 22 deletions(-) create mode 100644 tools/concurrencytest/.gitignore create mode 100644 tools/concurrencytest/README.md create mode 100644 tools/concurrencytest/concurrencytest.py

diff --git a/.travis.yml b/.travis.yml index ea3b20e0632..6bed41b5837 100644 --- a/.travis.yml +++ b/.travis.yml @@ -46,6 +46,7 @@ install: - virtualenv /tmp/venv - . /tmp/venv/bin/activate - pip install pytest + - pip install python-subunit - grub-mkimage -o ~/grub_x86.efi -O i386-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd - mkdir ~/grub2-arm - ( cd ~/grub2-arm; wget -O - http://download.opensuse.org/ports/armv7hl/distribution/leap/42.2/repo/oss/s... | rpm2cpio | cpio -di ) diff --git a/test/py/README.md b/test/py/README.md index aed2fd063a8..4d9d2b81d1e 100644 --- a/test/py/README.md +++ b/test/py/README.md @@ -29,6 +29,7 @@ tests. Similar package names should exist in other distributions. | -------------- | ----------------------------- | | python | 2.7.5-5ubuntu3 | | python-pytest | 2.5.1-1 | +| python-subunit | - | | gdisk | 0.8.8-1ubuntu0.1 | | dfu-util | 0.5-1 | | dtc | 1.4.0+dfsg-1 | diff --git a/tools/binman/binman.py b/tools/binman/binman.py index f5af5359f3c..439908e6650 100755 --- a/tools/binman/binman.py +++ b/tools/binman/binman.py @@ -10,6 +10,7 @@ """See README for more information"""

import glob +import multiprocessing import os import sys import traceback @@ -17,7 +18,7 @@ import unittest

# Bring in the patman and dtoc libraries our_path = os.path.dirname(os.path.realpath(__file__)) -for dirname in ['../patman', '../dtoc', '..']: +for dirname in ['../patman', '../dtoc', '..', '../concurrencytest']: sys.path.insert(0, os.path.join(our_path, dirname))

# Bring in the libfdt module @@ -27,16 +28,22 @@ sys.path.insert(0, os.path.join(our_path,

import cmdline import command +use_concurrent = True +try: + from concurrencytest import ConcurrentTestSuite, fork_for_tests +except: + use_concurrent = False import control import test_util

-def RunTests(debug, args): +def RunTests(debug, processes, args): """Run the functional tests and any embedded doctests

Args: debug: True to enable debugging, which shows a full stack trace on error args: List of positional args provided to binman. This can hold a test name to execute (as in 'binman -t testSections', for example) + processes: Number of processes to use to run tests (None=same as #CPUs) """ import elf_test import entry_test @@ -54,19 +61,28 @@ def RunTests(debug, args): sys.argv = [sys.argv[0]] if debug: sys.argv.append('-D') + if debug: + sys.argv.append('-D')

# Run the entry tests first ,since these need to be the first to import the # 'entry' module. test_name = args and args[0] or None + suite = unittest.TestSuite() + loader = unittest.TestLoader() for module in (entry_test.TestEntry, ftest.TestFunctional, fdt_test.TestFdt, elf_test.TestElf, image_test.TestImage): if test_name: try: - suite = unittest.TestLoader().loadTestsFromName(test_name, module) + suite.addTests(loader.loadTestsFromName(test_name, module)) except AttributeError: continue else: - suite = unittest.TestLoader().loadTestsFromTestCase(module) + suite.addTests(loader.loadTestsFromTestCase(module)) + if use_concurrent and processes != 1: + concurrent_suite = ConcurrentTestSuite(suite, + fork_for_tests(processes or multiprocessing.cpu_count())) + concurrent_suite.run(result) + else: suite.run(result)

print result @@ -115,7 +131,7 @@ def RunBinman(options, args): sys.tracebacklimit = 0

if options.test: - ret_code = RunTests(options.debug, args[1:]) + ret_code = RunTests(options.debug, options.processes, args[1:])

elif options.test_coverage: RunTestCoverage() diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index f8caa7d2841..3886d52b3a0 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -46,6 +46,8 @@ def ParseArgs(argv): parser.add_option('-p', '--preserve', action='store_true',\ help='Preserve temporary output directory even if option -O is not ' 'given') + parser.add_option('-P', '--processes', type=int, + help='set number of processes to use for running tests') parser.add_option('-t', '--test', action='store_true', default=False, help='run tests') parser.add_option('-T', '--test-coverage', action='store_true', diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py index a8bc938f9e9..17ab2290140 100644 --- a/tools/binman/entry_test.py +++ b/tools/binman/entry_test.py @@ -13,6 +13,8 @@ import fdt import fdt_util import tools

+entry = None + class TestEntry(unittest.TestCase): def setUp(self): tools.PrepareOutputDir(None) @@ -38,7 +40,10 @@ class TestEntry(unittest.TestCase): def test2EntryImportLib(self): del sys.modules['importlib'] global entry - reload(entry) + if entry: + reload(entry) + else: + import entry entry.Entry.Create(None, self.GetNode(), 'u-boot-spl') del entry

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index cbde8beb5ab..7598abd7f5b 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -365,6 +365,16 @@ class TestFunctional(unittest.TestCase): os.makedirs(pathname) return pathname

+ @classmethod + def _SetupSplElf(self, src_fname='bss_data'): + """Set up an ELF file with a '_dt_ucode_base_size' symbol + + Args: + Filename of ELF file to use as SPL + """ + with open(self.TestFile(src_fname)) as fd: + TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + @classmethod def TestFile(self, fname): return os.path.join(self._binman_dir, 'test', fname) @@ -516,9 +526,7 @@ class TestFunctional(unittest.TestCase): ucode_second: True if the microsecond entry is second instead of third """ - # ELF file with a '_dt_ucode_base_size' symbol - with open(self.TestFile('u_boot_ucode_ptr')) as fd: - TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + self._SetupSplElf('u_boot_ucode_ptr') first, pos_and_size = self._RunMicrocodeTest(dts, U_BOOT_SPL_NODTB_DATA, ucode_second=ucode_second) self.assertEqual('splnodtb with microc' + pos_and_size + @@ -818,8 +826,7 @@ class TestFunctional(unittest.TestCase):

def testImagePadByte(self): """Test that the image pad byte can be specified""" - with open(self.TestFile('bss_data')) as fd: - TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + self._SetupSplElf() data = self._DoReadFile('21_image_pad.dts') self.assertEqual(U_BOOT_SPL_DATA + (chr(0xff) * 1) + U_BOOT_DATA, data)

@@ -842,6 +849,7 @@ class TestFunctional(unittest.TestCase):

def testPackSorted(self): """Test that entries can be sorted""" + self._SetupSplElf() data = self._DoReadFile('24_sorted.dts') self.assertEqual(chr(0) * 1 + U_BOOT_SPL_DATA + chr(0) * 2 + U_BOOT_DATA, data) @@ -876,6 +884,7 @@ class TestFunctional(unittest.TestCase):

def testPackX86Rom(self): """Test that a basic x86 ROM can be created""" + self._SetupSplElf() data = self._DoReadFile('29_x86-rom.dts') self.assertEqual(U_BOOT_DATA + chr(0) * 7 + U_BOOT_SPL_DATA + chr(0) * 2, data) @@ -1023,15 +1032,13 @@ class TestFunctional(unittest.TestCase): def testSplBssPad(self): """Test that we can pad SPL's BSS with zeros""" # ELF file with a '__bss_size' symbol - with open(self.TestFile('bss_data')) as fd: - TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + self._SetupSplElf() data = self._DoReadFile('47_spl_bss_pad.dts') self.assertEqual(U_BOOT_SPL_DATA + (chr(0) * 10) + U_BOOT_DATA, data)

def testSplBssPadMissing(self): """Test that a missing symbol is detected""" - with open(self.TestFile('u_boot_ucode_ptr')) as fd: - TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + self._SetupSplElf('u_boot_ucode_ptr') with self.assertRaises(ValueError) as e: self._DoReadFile('47_spl_bss_pad.dts') self.assertIn('Expected __bss_size symbol in spl/u-boot-spl', @@ -1078,8 +1085,7 @@ class TestFunctional(unittest.TestCase): addr = elf.GetSymbolAddress(elf_fname, '__image_copy_start') self.assertEqual(syms['_binman_u_boot_spl_prop_offset'].address, addr)

- with open(self.TestFile('u_boot_binman_syms')) as fd: - TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + self._SetupSplElf('u_boot_binman_syms') data = self._DoReadFile('53_symbols.dts') sym_values = struct.pack('<LQL', 0x24 + 0, 0x24 + 24, 0x24 + 20) expected = (sym_values + U_BOOT_SPL_DATA[16:] + chr(0xff) + @@ -1710,16 +1716,14 @@ class TestFunctional(unittest.TestCase):

def testElf(self): """Basic test of ELF entries""" - with open(self.TestFile('bss_data')) as fd: - TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + self._SetupSplElf() with open(self.TestFile('bss_data')) as fd: TestFunctional._MakeInputFile('-boot', fd.read()) data = self._DoReadFile('96_elf.dts')

def testElfStripg(self): """Basic test of ELF entries""" - with open(self.TestFile('bss_data')) as fd: - TestFunctional._MakeInputFile('spl/u-boot-spl', fd.read()) + self._SetupSplElf() with open(self.TestFile('bss_data')) as fd: TestFunctional._MakeInputFile('-boot', fd.read()) data = self._DoReadFile('97_elf_strip.dts') diff --git a/tools/concurrencytest/.gitignore b/tools/concurrencytest/.gitignore new file mode 100644 index 00000000000..0d20b6487c6 --- /dev/null +++ b/tools/concurrencytest/.gitignore @@ -0,0 +1 @@ +*.pyc diff --git a/tools/concurrencytest/README.md b/tools/concurrencytest/README.md new file mode 100644 index 00000000000..8e65776f178 --- /dev/null +++ b/tools/concurrencytest/README.md @@ -0,0 +1,74 @@ +concurrencytest +=============== + + + +Python testtools extension for running unittest suites concurrently. + +---- + +Install from PyPI: +``` +pip install concurrencytest +``` + +---- + +Requires: + + * [testtools](https://pypi.python.org/pypi/testtools) : `pip install testtools` + * [python-subunit](https://pypi.python.org/pypi/python-subunit) : `pip install python-subunit` + +---- + +Example: + +```python +import time +import unittest + +from concurrencytest import ConcurrentTestSuite, fork_for_tests + + +class SampleTestCase(unittest.TestCase): + """Dummy tests that sleep for demo.""" + + def test_me_1(self): + time.sleep(0.5) + + def test_me_2(self): + time.sleep(0.5) + + def test_me_3(self): + time.sleep(0.5) + + def test_me_4(self): + time.sleep(0.5) + + +# Load tests from SampleTestCase defined above +suite = unittest.TestLoader().loadTestsFromTestCase(SampleTestCase) +runner = unittest.TextTestRunner() + +# Run tests sequentially +runner.run(suite) + +# Run same tests across 4 processes +suite = unittest.TestLoader().loadTestsFromTestCase(SampleTestCase) +concurrent_suite = ConcurrentTestSuite(suite, fork_for_tests(4)) +runner.run(concurrent_suite) +``` +Output: + +``` +.... +---------------------------------------------------------------------- +Ran 4 tests in 2.003s + +OK +.... +---------------------------------------------------------------------- +Ran 4 tests in 0.504s + +OK +``` diff --git a/tools/concurrencytest/concurrencytest.py b/tools/concurrencytest/concurrencytest.py new file mode 100644 index 00000000000..418d7eed21d --- /dev/null +++ b/tools/concurrencytest/concurrencytest.py @@ -0,0 +1,144 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: GPL-2.0+ +# +# Modified by: Corey Goldberg, 2013 +# +# Original code from: +# Bazaar (bzrlib.tests.__init__.py, v2.6, copied Jun 01 2013) +# Copyright (C) 2005-2011 Canonical Ltd + +"""Python testtools extension for running unittest suites concurrently. + +The `testtools` project provides a ConcurrentTestSuite class, but does +not provide a `make_tests` implementation needed to use it. + +This allows you to parallelize a test run across a configurable number +of worker processes. While this can speed up CPU-bound test runs, it is +mainly useful for IO-bound tests that spend most of their time waiting for +data to arrive from someplace else and can benefit from cocncurrency. + +Unix only. +""" + +import os +import sys +import traceback +import unittest +from itertools import cycle +from multiprocessing import cpu_count + +from subunit import ProtocolTestCase, TestProtocolClient +from subunit.test_results import AutoTimingTestResultDecorator + +from testtools import ConcurrentTestSuite, iterate_tests + + +_all__ = [ + 'ConcurrentTestSuite', + 'fork_for_tests', + 'partition_tests', +] + + +CPU_COUNT = cpu_count() + + +def fork_for_tests(concurrency_num=CPU_COUNT): + """Implementation of `make_tests` used to construct `ConcurrentTestSuite`. + + :param concurrency_num: number of processes to use. + """ + def do_fork(suite): + """Take suite and start up multiple runners by forking (Unix only). + + :param suite: TestSuite object. + + :return: An iterable of TestCase-like objects which can each have + run(result) called on them to feed tests to result. + """ + result = [] + test_blocks = partition_tests(suite, concurrency_num) + # Clear the tests from the original suite so it doesn't keep them alive + suite._tests[:] = [] + for process_tests in test_blocks: + process_suite = unittest.TestSuite(process_tests) + # Also clear each split list so new suite has only reference + process_tests[:] = [] + c2pread, c2pwrite = os.pipe() + pid = os.fork() + if pid == 0: + try: + stream = os.fdopen(c2pwrite, 'wb', 1) + os.close(c2pread) + # Leave stderr and stdout open so we can see test noise + # Close stdin so that the child goes away if it decides to + # read from stdin (otherwise its a roulette to see what + # child actually gets keystrokes for pdb etc). + sys.stdin.close() + subunit_result = AutoTimingTestResultDecorator( + TestProtocolClient(stream) + ) + process_suite.run(subunit_result) + except: + # Try and report traceback on stream, but exit with error + # even if stream couldn't be created or something else + # goes wrong. The traceback is formatted to a string and + # written in one go to avoid interleaving lines from + # multiple failing children. + try: + stream.write(traceback.format_exc()) + finally: + os._exit(1) + os._exit(0) + else: + os.close(c2pwrite) + stream = os.fdopen(c2pread, 'rb', 1) + test = ProtocolTestCase(stream) + result.append(test) + return result + return do_fork + + +def partition_tests(suite, count): + """Partition suite into count lists of tests.""" + # This just assigns tests in a round-robin fashion. On one hand this + # splits up blocks of related tests that might run faster if they shared + # resources, but on the other it avoids assigning blocks of slow tests to + # just one partition. So the slowest partition shouldn't be much slower + # than the fastest. + partitions = [list() for _ in range(count)] + tests = iterate_tests(suite) + for partition, test in zip(cycle(partitions), tests): + partition.append(test) + return partitions + + +if __name__ == '__main__': + import time + + class SampleTestCase(unittest.TestCase): + """Dummy tests that sleep for demo.""" + + def test_me_1(self): + time.sleep(0.5) + + def test_me_2(self): + time.sleep(0.5) + + def test_me_3(self): + time.sleep(0.5) + + def test_me_4(self): + time.sleep(0.5) + + # Load tests from SampleTestCase defined above + suite = unittest.TestLoader().loadTestsFromTestCase(SampleTestCase) + runner = unittest.TextTestRunner() + + # Run tests sequentially + runner.run(suite) + + # Run same tests across 4 processes + suite = unittest.TestLoader().loadTestsFromTestCase(SampleTestCase) + concurrent_suite = ConcurrentTestSuite(suite, fork_for_tests(4)) + runner.run(concurrent_suite) diff --git a/tools/dtoc/dtoc.py b/tools/dtoc/dtoc.py index 33b2589c561..2277af9bf78 100755 --- a/tools/dtoc/dtoc.py +++ b/tools/dtoc/dtoc.py @@ -89,6 +89,8 @@ parser.add_option('--include-disabled', action='store_true', help='Include disabled nodes') parser.add_option('-o', '--output', action='store', default='-', help='Select output filename') +parser.add_option('-P', '--processes', type=int, + help='set number of processes to use for running tests') parser.add_option('-t', '--test', action='store_true', dest='test', default=False, help='run tests') parser.add_option('-T', '--test-coverage', action='store_true', diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py index 2e6febe8f38..8d70dd2a294 100755 --- a/tools/dtoc/test_fdt.py +++ b/tools/dtoc/test_fdt.py @@ -547,6 +547,8 @@ if __name__ != '__main__': parser = OptionParser() parser.add_option('-B', '--build-dir', type='string', default='b', help='Directory containing the build output') +parser.add_option('-P', '--processes', type=int, + help='set number of processes to use for running tests') parser.add_option('-t', '--test', action='store_true', dest='test', default=False, help='run tests') parser.add_option('-T', '--test-coverage', action='store_true', diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index 0e79af871ab..687d40704ab 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -43,7 +43,7 @@ def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None): glob_list += exclude_list glob_list += ['*libfdt.py', '*site-packages*'] cmd = ('PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools python-coverage run ' - '--omit "%s" %s -t' % (build_dir, ','.join(glob_list), prog)) + '--omit "%s" %s -P1 -t' % (build_dir, ','.join(glob_list), prog)) os.system(cmd) stdout = command.Output('python-coverage', 'report') lines = stdout.splitlines()