All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage
@ 2014-09-06  1:00 Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 01/18] patman: Add a way of recording terminal output for testing Simon Glass
                   ` (17 more replies)
  0 siblings, 18 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

Buildman's test coverage is inadequate, particularly in the area of the
core builder threads and logic. As a result it is harder to make changes
than it should be, since verifying correctness manually is tedious.

The existing output test relies on the user to verify that things look
OK. This is getting harder with more output options available, so this
series turns it into a unit test.

A new functional test is provided which tests buildman from the parser
options down to the logic that issues git and make commands. This runs
in a few seconds and provides coverage of the builder logic and threads,
plus most build-related options.

Output formatting is already tested in test.py, and there is also a test
there which checks that errors and warnings are correctly detected by the
build system and reported in the summary.

So overall, with this series, test coverage is now considerably better.

This test also fixes a few bugs, some reported by Steve Rae:
- The -H options hang
- An incorrect 'failure' count value in some cases
- Correct GetMetaDataForList() action
- Permit '/' in branch names
- Ignore conflicting tags (allows building multiple series together)

Changes in v3:
- Add new patch to permit branch names with an embedded '/'
- Add new patch to ignore conflicting tags in buildman

Changes in v2:
- Add a function to print out the terminal output recorded
- Add a comment to _HandleCommandGit
- Make sure the test temporary directory is removed
- Add patch to expand output test to cover directory prefixes

Simon Glass (18):
  patman: Add a way of recording terminal output for testing
  buildman: Send builder output through a function for testing
  buildman: Enhance basic test to check summary output
  patman: RunPipe() should not pipe stdout/stderr unless asked
  buildman: Move the command line code into its own file
  buildman: Move full help code into the control module
  patman: Provide a way to intercept commands for testing
  buildman: Add a functional test
  buildman: Set up bsettings outside the control module
  buildman: Avoid looking at config file or toolchains in tests
  buildman: Allow tests to have their own boards
  buildman: Correct counting of build failures on retry
  buildman: Provide an internal option to clean the outpur dir
  patman: Start with a clean series when needed
  buildman: Add additional functional tests
  buildman: Expand output test to cover directory prefixes
  buildman: Permit branch names with an embedded '/'
  buildman: Ignore conflicting tags

 tools/buildman/bsettings.py     |  15 +-
 tools/buildman/builder.py       |  58 ++---
 tools/buildman/builderthread.py |  15 +-
 tools/buildman/buildman.py      |  98 +-------
 tools/buildman/cmdline.py       |  85 +++++++
 tools/buildman/control.py       |  73 ++++--
 tools/buildman/func_test.py     | 519 ++++++++++++++++++++++++++++++++++++++++
 tools/buildman/test.py          | 153 +++++++++++-
 tools/buildman/toolchain.py     |   4 +-
 tools/patman/command.py         |  22 ++
 tools/patman/patchstream.py     |   6 +-
 tools/patman/terminal.py        |  72 ++++++
 12 files changed, 955 insertions(+), 165 deletions(-)
 create mode 100644 tools/buildman/cmdline.py
 create mode 100644 tools/buildman/func_test.py

-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 01/18] patman: Add a way of recording terminal output for testing
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:48   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 02/18] buildman: Send builder output through a function " Simon Glass
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

When running unit tests we don't want output to go to the terminal.
Provide a way of collecting it so that it can be examined by test code
later.

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

Changes in v3: None
Changes in v2:
- Add a function to print out the terminal output recorded

 tools/patman/terminal.py | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py
index 963f2f8..e78a7c1 100644
--- a/tools/patman/terminal.py
+++ b/tools/patman/terminal.py
@@ -14,6 +14,78 @@ import sys
 # Selection of when we want our output to be colored
 COLOR_IF_TERMINAL, COLOR_ALWAYS, COLOR_NEVER = range(3)
 
+# Initially, we are set up to print to the terminal
+print_test_mode = False
+print_test_list = []
+
+class PrintLine:
+    """A line of text output
+
+    Members:
+        text: Text line that was printed
+        newline: True to output a newline after the text
+        colour: Text colour to use
+    """
+    def __init__(self, text, newline, colour):
+        self.text = text
+        self.newline = newline
+        self.colour = colour
+
+    def __str__(self):
+        return 'newline=%s, colour=%s, text=%s' % (self.newline, self.colour,
+                self.text)
+
+def Print(text='', newline=True, colour=None):
+    """Handle a line of output to the terminal.
+
+    In test mode this is recorded in a list. Otherwise it is output to the
+    terminal.
+
+    Args:
+        text: Text to print
+        newline: True to add a new line at the end of the text
+        colour: Colour to use for the text
+    """
+    if print_test_mode:
+        print_test_list.append(PrintLine(text, newline, colour))
+    else:
+        if colour:
+            col = Color()
+            text = col.Color(colour, text)
+        print text,
+        if newline:
+            print
+
+def SetPrintTestMode():
+    """Go into test mode, where all printing is recorded"""
+    global print_test_mode
+
+    print_test_mode = True
+
+def GetPrintTestLines():
+    """Get a list of all lines output through Print()
+
+    Returns:
+        A list of PrintLine objects
+    """
+    global print_test_list
+
+    ret = print_test_list
+    print_test_list = []
+    return ret
+
+def EchoPrintTestLines():
+    """Print out the text lines collected"""
+    for line in print_test_list:
+        if line.colour:
+            col = Color()
+            print col.Color(line.colour, line.text),
+        else:
+            print line.text,
+        if line.newline:
+            print
+
+
 class Color(object):
     """Conditionally wraps text in ANSI color escape sequences."""
     BLACK, RED, GREEN, YELLOW, BLUE, MAGENTA, CYAN, WHITE = range(8)
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 02/18] buildman: Send builder output through a function for testing
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 01/18] patman: Add a way of recording terminal output for testing Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:49   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 03/18] buildman: Enhance basic test to check summary output Simon Glass
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

To allow us to verify the builder's console output, send it through a
function which can collect it when running in test mode.

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

Changes in v3: None
Changes in v2: None

 tools/buildman/builder.py | 58 ++++++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 324239a..1b6517b 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -20,6 +20,7 @@ import builderthread
 import command
 import gitutil
 import terminal
+from terminal import Print
 import toolchain
 
 
@@ -299,8 +300,8 @@ class Builder:
             length: Length of new line, in characters
         """
         if length < self.last_line_len:
-            print ' ' * (self.last_line_len - length),
-            print '\r',
+            Print(' ' * (self.last_line_len - length), newline=False)
+            Print('\r', newline=False)
         self.last_line_len = length
         sys.stdout.flush()
 
@@ -351,7 +352,7 @@ class Builder:
             if result.already_done:
                 self.already_done += 1
             if self._verbose:
-                print '\r',
+                Print('\r', newline=False)
                 self.ClearLine(0)
                 boards_selected = {target : result.brd}
                 self.ResetResultSummary(boards_selected)
@@ -379,7 +380,7 @@ class Builder:
                     self.commit_count)
 
         name += target
-        print line + name,
+        Print(line + name, newline=False)
         length = 14 + len(name)
         self.ClearLine(length)
 
@@ -495,7 +496,7 @@ class Builder:
             try:
                 size, type, name = line[:-1].split()
             except:
-                print "Invalid line in file '%s': '%s'" % (fname, line[:-1])
+                Print("Invalid line in file '%s': '%s'" % (fname, line[:-1]))
                 continue
             if type in 'tTdDbB':
                 # function names begin with '.' on 64-bit powerpc
@@ -723,16 +724,16 @@ class Builder:
             return
         args = [self.ColourNum(x) for x in args]
         indent = ' ' * 15
-        print ('%s%s: add: %s/%s, grow: %s/%s bytes: %s/%s (%s)' %
-               tuple([indent, self.col.Color(self.col.YELLOW, fname)] + args))
-        print '%s  %-38s %7s %7s %+7s' % (indent, 'function', 'old', 'new',
-                                        'delta')
+        Print('%s%s: add: %s/%s, grow: %s/%s bytes: %s/%s (%s)' %
+              tuple([indent, self.col.Color(self.col.YELLOW, fname)] + args))
+        Print('%s  %-38s %7s %7s %+7s' % (indent, 'function', 'old', 'new',
+                                         'delta'))
         for diff, name in delta:
             if diff:
                 color = self.col.RED if diff > 0 else self.col.GREEN
                 msg = '%s  %-38s %7s %7s %+7d' % (indent, name,
                         old.get(name, '-'), new.get(name,'-'), diff)
-                print self.col.Color(color, msg)
+                Print(msg, colour=color)
 
 
     def PrintSizeDetail(self, target_list, show_bloat):
@@ -757,11 +758,12 @@ class Builder:
                     color = self.col.RED if diff > 0 else self.col.GREEN
                 msg = ' %s %+d' % (name, diff)
                 if not printed_target:
-                    print '%10s  %-15s:' % ('', result['_target']),
+                    Print('%10s  %-15s:' % ('', result['_target']),
+                          newline=False)
                     printed_target = True
-                print self.col.Color(color, msg),
+                Print(msg, colour=color, newline=False)
             if printed_target:
-                print
+                Print()
                 if show_bloat:
                     target = result['_target']
                     outcome = result['_outcome']
@@ -866,13 +868,13 @@ class Builder:
                     color = self.col.RED if avg_diff > 0 else self.col.GREEN
                     msg = ' %s %+1.1f' % (name, avg_diff)
                     if not printed_arch:
-                        print '%10s: (for %d/%d boards)' % (arch, count,
-                                arch_count[arch]),
+                        Print('%10s: (for %d/%d boards)' % (arch, count,
+                              arch_count[arch]), newline=False)
                         printed_arch = True
-                    print self.col.Color(color, msg),
+                    Print(msg, colour=color, newline=False)
 
             if printed_arch:
-                print
+                Print()
                 if show_detail:
                     self.PrintSizeDetail(target_list, show_bloat)
 
@@ -977,19 +979,19 @@ class Builder:
                 self.AddOutcome(board_selected, arch_list, unknown, '?',
                         self.col.MAGENTA)
             for arch, target_list in arch_list.iteritems():
-                print '%10s: %s' % (arch, target_list)
+                Print('%10s: %s' % (arch, target_list))
                 self._error_lines += 1
             if better_err:
-                print self.col.Color(self.col.GREEN, '\n'.join(better_err))
+                Print('\n'.join(better_err), colour=self.col.GREEN)
                 self._error_lines += 1
             if worse_err:
-                print self.col.Color(self.col.RED, '\n'.join(worse_err))
+                Print('\n'.join(worse_err), colour=self.col.RED)
                 self._error_lines += 1
             if better_warn:
-                print self.col.Color(self.col.YELLOW, '\n'.join(better_warn))
+                Print('\n'.join(better_warn), colour=self.col.CYAN)
                 self._error_lines += 1
             if worse_warn:
-                print self.col.Color(self.col.MAGENTA, '\n'.join(worse_warn))
+                Print('\n'.join(worse_warn), colour=self.col.MAGENTA)
                 self._error_lines += 1
 
         if show_sizes:
@@ -1009,8 +1011,8 @@ class Builder:
             if not board in board_dict:
                 not_built.append(board)
         if not_built:
-            print "Boards not built (%d): %s" % (len(not_built),
-                    ', '.join(not_built))
+            Print("Boards not built (%d): %s" % (len(not_built),
+                  ', '.join(not_built)))
 
     def ProduceResultSummary(self, commit_upto, commits, board_selected):
             (board_dict, err_lines, err_line_boards, warn_lines,
@@ -1020,7 +1022,7 @@ class Builder:
             if commits:
                 msg = '%02d: %s' % (commit_upto + 1,
                         commits[commit_upto].subject)
-                print self.col.Color(self.col.BLUE, msg)
+                Print(msg, colour=self.col.BLUE)
             self.PrintResultSummary(board_selected, board_dict,
                     err_lines if self._show_errors else [], err_line_boards,
                     warn_lines if self._show_errors else [], warn_line_boards,
@@ -1044,7 +1046,7 @@ class Builder:
         for commit_upto in range(0, self.commit_count, self._step):
             self.ProduceResultSummary(commit_upto, commits, board_selected)
         if not self._error_lines:
-            print self.col.Color(self.col.GREEN, '(no errors to report)')
+            Print('(no errors to report)', colour=self.col.GREEN)
 
 
     def SetupBuild(self, board_selected, commits):
@@ -1089,7 +1091,7 @@ class Builder:
             if os.path.exists(git_dir):
                 gitutil.Fetch(git_dir, thread_dir)
             else:
-                print 'Cloning repo for thread %d' % thread_num
+                Print('Cloning repo for thread %d' % thread_num)
                 gitutil.Clone(src_dir, thread_dir)
 
     def _PrepareWorkingSpace(self, max_threads, setup_git):
@@ -1160,6 +1162,6 @@ class Builder:
 
         # Wait until we have processed all output
         self.out_queue.join()
-        print
+        Print()
         self.ClearLine(0)
         return (self.fail, self.warned)
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 03/18] buildman: Enhance basic test to check summary output
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 01/18] patman: Add a way of recording terminal output for testing Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 02/18] buildman: Send builder output through a function " Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:49   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 04/18] patman: RunPipe() should not pipe stdout/stderr unless asked Simon Glass
                   ` (14 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

Adjust the basic test so that it checks all console output. This will help
to ensure that the builder is behaving correctly with printing summary
information.

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

Changes in v3: None
Changes in v2: None

 tools/buildman/test.py | 101 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 96 insertions(+), 5 deletions(-)

diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index a51c942..f0c4d0e 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -21,20 +21,21 @@ import builder
 import control
 import command
 import commit
+import terminal
 import toolchain
 
 errors = [
     '''main.c: In function 'main_loop':
 main.c:260:6: warning: unused variable 'joe' [-Wunused-variable]
 ''',
-    '''main.c: In function 'main_loop':
+    '''main.c: In function 'main_loop2':
 main.c:295:2: error: 'fred' undeclared (first use in this function)
 main.c:295:2: note: each undeclared identifier is reported only once for each function it appears in
 make[1]: *** [main.o] Error 1
 make: *** [common/libcommon.o] Error 2
 Make failed
 ''',
-    '''main.c: In function 'main_loop':
+    '''main.c: In function 'main_loop3':
 main.c:280:6: warning: unused variable 'mary' [-Wunused-variable]
 ''',
     '''powerpc-linux-ld: warning: dot moved backwards before `.bss'
@@ -103,6 +104,10 @@ class TestBuild(unittest.TestCase):
         self.toolchains.Add('powerpc-linux-gcc', test=False)
         self.toolchains.Add('gcc', test=False)
 
+        # Avoid sending any output
+        terminal.SetPrintTestMode()
+        self._col = terminal.Color()
+
     def Make(self, commit, brd, stage, *args, **kwargs):
         result = command.CommandResult()
         boardnum = int(brd.target[-1])
@@ -121,13 +126,26 @@ class TestBuild(unittest.TestCase):
 
             if not os.path.isdir(target_dir):
                 os.mkdir(target_dir)
-            #time.sleep(.2 + boardnum * .2)
 
         result.combined = result.stdout + result.stderr
         return result
 
-    def testBasic(self):
-        """Test basic builder operation"""
+    def assertSummary(self, text, arch, plus, boards, ok=False):
+        col = self._col
+        expected_colour = col.GREEN if ok else col.RED
+        expect = '%10s: ' % arch
+        # TODO(sjg at chromium.org): If plus is '', we shouldn't need this
+        expect += col.Color(expected_colour, plus)
+        expect += '  '
+        for board in boards:
+            expect += col.Color(expected_colour, ' %s' % board)
+        self.assertEqual(text, expect)
+
+    def testOutput(self):
+        """Test basic builder operation and output
+
+        This does a line-by-line verification of the summary output.
+        """
         output_dir = tempfile.mkdtemp()
         if not os.path.isdir(output_dir):
             os.mkdir(output_dir)
@@ -138,8 +156,81 @@ class TestBuild(unittest.TestCase):
 
         build.BuildBoards(self.commits, board_selected, keep_outputs=False,
                           verbose=False)
+        lines = terminal.GetPrintTestLines()
+        count = 0
+        for line in lines:
+            if line.text.strip():
+                count += 1
+
+        # We should get one starting message, then an update for every commit
+        # built.
+        self.assertEqual(count, len(commits) * len(boards) + 1)
         build.SetDisplayOptions(show_errors=True);
         build.ShowSummary(self.commits, board_selected)
+        lines = terminal.GetPrintTestLines()
+        self.assertEqual(lines[0].text, '01: %s' % commits[0][1])
+        self.assertEqual(lines[1].text, '02: %s' % commits[1][1])
+
+        # We expect all archs to fail
+        col = terminal.Color()
+        self.assertSummary(lines[2].text, 'sandbox', '+', ['board4'])
+        self.assertSummary(lines[3].text, 'arm', '+', ['board1'])
+        self.assertSummary(lines[4].text, 'powerpc', '+', ['board2', 'board3'])
+
+        # Now we should have the compiler warning
+        self.assertEqual(lines[5].text, 'w+%s' %
+                errors[0].rstrip().replace('\n', '\nw+'))
+        self.assertEqual(lines[5].colour, col.MAGENTA)
+
+        self.assertEqual(lines[6].text, '03: %s' % commits[2][1])
+        self.assertSummary(lines[7].text, 'sandbox', '+', ['board4'])
+        self.assertSummary(lines[8].text, 'arm', '', ['board1'], ok=True)
+        self.assertSummary(lines[9].text, 'powerpc', '+', ['board2', 'board3'])
+
+        # Compiler error
+        self.assertEqual(lines[10].text, '+%s' %
+                errors[1].rstrip().replace('\n', '\n+'))
+
+        self.assertEqual(lines[11].text, '04: %s' % commits[3][1])
+        self.assertSummary(lines[12].text, 'sandbox', '', ['board4'], ok=True)
+        self.assertSummary(lines[13].text, 'powerpc', '', ['board2', 'board3'],
+                ok=True)
+
+        # Compile error fixed
+        self.assertEqual(lines[14].text, '-%s' %
+                errors[1].rstrip().replace('\n', '\n-'))
+        self.assertEqual(lines[14].colour, col.GREEN)
+
+        self.assertEqual(lines[15].text, 'w+%s' %
+                errors[2].rstrip().replace('\n', '\nw+'))
+        self.assertEqual(lines[15].colour, col.MAGENTA)
+
+        self.assertEqual(lines[16].text, '05: %s' % commits[4][1])
+        self.assertSummary(lines[17].text, 'sandbox', '+', ['board4'])
+        self.assertSummary(lines[18].text, 'powerpc', '', ['board3'], ok=True)
+
+        # The second line of errors[3] is a duplicate, so buildman will drop it
+        expect = errors[3].rstrip().split('\n')
+        expect = [expect[0]] + expect[2:]
+        self.assertEqual(lines[19].text, '+%s' %
+                '\n'.join(expect).replace('\n', '\n+'))
+
+        self.assertEqual(lines[20].text, 'w-%s' %
+                errors[2].rstrip().replace('\n', '\nw-'))
+
+        self.assertEqual(lines[21].text, '06: %s' % commits[5][1])
+        self.assertSummary(lines[22].text, 'sandbox', '', ['board4'], ok=True)
+
+        # The second line of errors[3] is a duplicate, so buildman will drop it
+        expect = errors[3].rstrip().split('\n')
+        expect = [expect[0]] + expect[2:]
+        self.assertEqual(lines[23].text, '-%s' %
+                '\n'.join(expect).replace('\n', '\n-'))
+
+        self.assertEqual(lines[24].text, 'w-%s' %
+                errors[0].rstrip().replace('\n', '\nw-'))
+
+        self.assertEqual(len(lines), 25)
 
     def _testGit(self):
         """Test basic builder operation by building a branch"""
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 04/18] patman: RunPipe() should not pipe stdout/stderr unless asked
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (2 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 03/18] buildman: Enhance basic test to check summary output Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:49   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 05/18] buildman: Move the command line code into its own file Simon Glass
                   ` (13 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

RunPipe() currently pipes the output of stdout and stderr to a pty, but
this is not the intended behaviour. Fix it.

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

Changes in v3: None
Changes in v2: None

 tools/patman/command.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/patman/command.py b/tools/patman/command.py
index 449d3d0..7212fdf 100644
--- a/tools/patman/command.py
+++ b/tools/patman/command.py
@@ -48,6 +48,8 @@ def RunPipe(pipe_list, infile=None, outfile=None,
     last_pipe = None
     pipeline = list(pipe_list)
     user_pipestr =  '|'.join([' '.join(pipe) for pipe in pipe_list])
+    kwargs['stdout'] = None
+    kwargs['stderr'] = None
     while pipeline:
         cmd = pipeline.pop(0)
         if last_pipe is not None:
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 05/18] buildman: Move the command line code into its own file
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (3 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 04/18] patman: RunPipe() should not pipe stdout/stderr unless asked Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:50   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 06/18] buildman: Move full help code into the control module Simon Glass
                   ` (12 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

We want to be able to issue parser commands from within buildman for test
purposes. Move the parser code into its own file so we don't end up needing
the buildman and test modules to reference each other.

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

Changes in v3: None
Changes in v2: None

 tools/buildman/buildman.py | 73 ++-------------------------------------
 tools/buildman/cmdline.py  | 85 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+), 71 deletions(-)
 create mode 100644 tools/buildman/cmdline.py

diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py
index 1258b76..c4de857 100755
--- a/tools/buildman/buildman.py
+++ b/tools/buildman/buildman.py
@@ -8,7 +8,6 @@
 """See README for more information"""
 
 import multiprocessing
-from optparse import OptionParser
 import os
 import re
 import sys
@@ -22,7 +21,7 @@ sys.path.append(os.path.join(our_path, '../patman'))
 import board
 import builder
 import checkpatch
-import command
+import cmdline
 import control
 import doctest
 import gitutil
@@ -59,75 +58,7 @@ def RunTests():
         print err
 
 
-parser = OptionParser()
-parser.add_option('-b', '--branch', type='string',
-       help='Branch name to build')
-parser.add_option('-B', '--bloat', dest='show_bloat',
-       action='store_true', default=False,
-       help='Show changes in function code size for each board')
-parser.add_option('-c', '--count', dest='count', type='int',
-       default=-1, help='Run build on the top n commits')
-parser.add_option('-C', '--force-reconfig', dest='force_reconfig',
-       action='store_true', default=False,
-       help='Reconfigure for every commit (disable incremental build)')
-parser.add_option('-d', '--detail', dest='show_detail',
-       action='store_true', default=False,
-       help='Show detailed information for each board in summary')
-parser.add_option('-e', '--show_errors', action='store_true',
-       default=False, help='Show errors and warnings')
-parser.add_option('-f', '--force-build', dest='force_build',
-       action='store_true', default=False,
-       help='Force build of boards even if already built')
-parser.add_option('-F', '--force-build-failures', dest='force_build_failures',
-       action='store_true', default=False,
-       help='Force build of previously-failed build')
-parser.add_option('-g', '--git', type='string',
-       help='Git repo containing branch to build', default='.')
-parser.add_option('-G', '--config-file', type='string',
-       help='Path to buildman config file', default='')
-parser.add_option('-H', '--full-help', action='store_true', dest='full_help',
-       default=False, help='Display the README file')
-parser.add_option('-i', '--in-tree', dest='in_tree',
-       action='store_true', default=False,
-       help='Build in the source tree instead of a separate directory')
-parser.add_option('-j', '--jobs', dest='jobs', type='int',
-       default=None, help='Number of jobs to run at once (passed to make)')
-parser.add_option('-k', '--keep-outputs', action='store_true',
-       default=False, help='Keep all build output files (e.g. binaries)')
-parser.add_option('-l', '--list-error-boards', action='store_true',
-       default=False, help='Show a list of boards next to each error/warning')
-parser.add_option('--list-tool-chains', action='store_true', default=False,
-       help='List available tool chains')
-parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
-       default=False, help="Do a try run (describe actions, but no nothing)")
-parser.add_option('-o', '--output-dir', type='string',
-       dest='output_dir', default='..',
-       help='Directory where all builds happen and buildman has its workspace (default is ../)')
-parser.add_option('-Q', '--quick', action='store_true',
-       default=False, help='Do a rough build, with limited warning resolution')
-parser.add_option('-s', '--summary', action='store_true',
-       default=False, help='Show a build summary')
-parser.add_option('-S', '--show-sizes', action='store_true',
-       default=False, help='Show image size variation in summary')
-parser.add_option('--step', type='int',
-       default=1, help='Only build every n commits (0=just first and last)')
-parser.add_option('-t', '--test', action='store_true', dest='test',
-                  default=False, help='run tests')
-parser.add_option('-T', '--threads', type='int',
-       default=None, help='Number of builder threads to use')
-parser.add_option('-u', '--show_unknown', action='store_true',
-       default=False, help='Show boards with unknown build result')
-parser.add_option('-v', '--verbose', action='store_true',
-       default=False, help='Show build results while the build progresses')
-parser.add_option('-x', '--exclude', dest='exclude',
-      type='string', action='append',
-      help='Specify a list of boards to exclude, separated by comma')
-
-parser.usage += """
-
-Build U-Boot for all commits in a branch. Use -n to do a dry run"""
-
-(options, args) = parser.parse_args()
+options, args = cmdline.ParseArgs()
 
 # Run our meagre tests
 if options.test:
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
new file mode 100644
index 0000000..fad9a1c
--- /dev/null
+++ b/tools/buildman/cmdline.py
@@ -0,0 +1,85 @@
+#
+# Copyright (c) 2014 Google, Inc
+#
+# SPDX-License-Identifier:      GPL-2.0+
+#
+
+from optparse import OptionParser
+
+def ParseArgs():
+    """Parse command line arguments from sys.argv[]
+
+    Returns:
+        tuple containing:
+            options: command line options
+            args: command lin arguments
+    """
+    parser = OptionParser()
+    parser.add_option('-b', '--branch', type='string',
+          help='Branch name to build')
+    parser.add_option('-B', '--bloat', dest='show_bloat',
+          action='store_true', default=False,
+          help='Show changes in function code size for each board')
+    parser.add_option('-c', '--count', dest='count', type='int',
+          default=-1, help='Run build on the top n commits')
+    parser.add_option('-C', '--force-reconfig', dest='force_reconfig',
+          action='store_true', default=False,
+          help='Reconfigure for every commit (disable incremental build)')
+    parser.add_option('-d', '--detail', dest='show_detail',
+          action='store_true', default=False,
+          help='Show detailed information for each board in summary')
+    parser.add_option('-e', '--show_errors', action='store_true',
+          default=False, help='Show errors and warnings')
+    parser.add_option('-f', '--force-build', dest='force_build',
+          action='store_true', default=False,
+          help='Force build of boards even if already built')
+    parser.add_option('-F', '--force-build-failures', dest='force_build_failures',
+          action='store_true', default=False,
+          help='Force build of previously-failed build')
+    parser.add_option('-g', '--git', type='string',
+          help='Git repo containing branch to build', default='.')
+    parser.add_option('-G', '--config-file', type='string',
+          help='Path to buildman config file', default='')
+    parser.add_option('-H', '--full-help', action='store_true', dest='full_help',
+          default=False, help='Display the README file')
+    parser.add_option('-i', '--in-tree', dest='in_tree',
+          action='store_true', default=False,
+          help='Build in the source tree instead of a separate directory')
+    parser.add_option('-j', '--jobs', dest='jobs', type='int',
+          default=None, help='Number of jobs to run at once (passed to make)')
+    parser.add_option('-k', '--keep-outputs', action='store_true',
+          default=False, help='Keep all build output files (e.g. binaries)')
+    parser.add_option('-l', '--list-error-boards', action='store_true',
+          default=False, help='Show a list of boards next to each error/warning')
+    parser.add_option('--list-tool-chains', action='store_true', default=False,
+          help='List available tool chains')
+    parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
+          default=False, help="Do a try run (describe actions, but no nothing)")
+    parser.add_option('-o', '--output-dir', type='string',
+          dest='output_dir', default='..',
+          help='Directory where all builds happen and buildman has its workspace (default is ../)')
+    parser.add_option('-Q', '--quick', action='store_true',
+          default=False, help='Do a rough build, with limited warning resolution')
+    parser.add_option('-s', '--summary', action='store_true',
+          default=False, help='Show a build summary')
+    parser.add_option('-S', '--show-sizes', action='store_true',
+          default=False, help='Show image size variation in summary')
+    parser.add_option('--step', type='int',
+          default=1, help='Only build every n commits (0=just first and last)')
+    parser.add_option('-t', '--test', action='store_true', dest='test',
+                      default=False, help='run tests')
+    parser.add_option('-T', '--threads', type='int',
+          default=None, help='Number of builder threads to use')
+    parser.add_option('-u', '--show_unknown', action='store_true',
+          default=False, help='Show boards with unknown build result')
+    parser.add_option('-v', '--verbose', action='store_true',
+          default=False, help='Show build results while the build progresses')
+    parser.add_option('-x', '--exclude', dest='exclude',
+          type='string', action='append',
+          help='Specify a list of boards to exclude, separated by comma')
+
+    parser.usage += """
+
+    Build U-Boot for all commits in a branch. Use -n to do a dry run"""
+
+    return parser.parse_args()
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 06/18] buildman: Move full help code into the control module
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (4 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 05/18] buildman: Move the command line code into its own file Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:50   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 07/18] patman: Provide a way to intercept commands for testing Simon Glass
                   ` (11 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

There is no good reason to keep this code separate. Move it into control.py
so it is easier to test.

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

Changes in v3: None
Changes in v2: None

 tools/buildman/buildman.py | 6 ------
 tools/buildman/control.py  | 8 ++++++++
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py
index c4de857..70c2142 100755
--- a/tools/buildman/buildman.py
+++ b/tools/buildman/buildman.py
@@ -63,12 +63,6 @@ options, args = cmdline.ParseArgs()
 # Run our meagre tests
 if options.test:
     RunTests()
-elif options.full_help:
-    pager = os.getenv('PAGER')
-    if not pager:
-        pager = 'more'
-    fname = os.path.join(os.path.dirname(sys.argv[0]), 'README')
-    command.Run(pager, fname)
 
 # Build selected commits for selected boards
 else:
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 06c9229..408d9b1 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -84,6 +84,14 @@ def DoBuildman(options, args):
         options: Command line options object
         args: Command line arguments (list of strings)
     """
+    if options.full_help:
+        pager = os.getenv('PAGER')
+        if not pager:
+            pager = 'more'
+        fname = os.path.join(os.path.dirname(sys.argv[0]), 'README')
+        command.Run(pager, fname)
+        return 0
+
     gitutil.Setup()
 
     bsettings.Setup(options.config_file)
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 07/18] patman: Provide a way to intercept commands for testing
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (5 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 06/18] buildman: Move full help code into the control module Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:50   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 08/18] buildman: Add a functional test Simon Glass
                   ` (10 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

Add a test point for the command module. This allows tests to emulate
the execution of commands. This provides more control (since we can make
the fake 'commands' do whatever we like), makes it faster to write tests
since we don't need to set up as much environment, and speeds up test
execution.

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

Changes in v3: None
Changes in v2: None

 tools/patman/command.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tools/patman/command.py b/tools/patman/command.py
index 7212fdf..d586f11 100644
--- a/tools/patman/command.py
+++ b/tools/patman/command.py
@@ -20,9 +20,25 @@ class CommandResult:
     def __init__(self):
         self.stdout = None
         self.stderr = None
+        self.combined = None
         self.return_code = None
         self.exception = None
 
+    def __init__(self, stdout='', stderr='', combined='', return_code=0,
+                 exception=None):
+        self.stdout = stdout
+        self.stderr = stderr
+        self.combined = combined
+        self.return_code = return_code
+        self.exception = exception
+
+
+# This permits interception of RunPipe for test purposes. If it is set to
+# a function, then that function is called with the pipe list being
+# executed. Otherwise, it is assumed to be a CommandResult object, and is
+# returned as the result for every RunPipe() call.
+# When this value is None, commands are executed as normal.
+test_result = None
 
 def RunPipe(pipe_list, infile=None, outfile=None,
             capture=False, capture_stderr=False, oneline=False,
@@ -44,6 +60,10 @@ def RunPipe(pipe_list, infile=None, outfile=None,
     Returns:
         CommandResult object
     """
+    if test_result:
+        if hasattr(test_result, '__call__'):
+            return test_result(pipe_list=pipe_list)
+        return test_result
     result = CommandResult()
     last_pipe = None
     pipeline = list(pipe_list)
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 08/18] buildman: Add a functional test
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (6 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 07/18] patman: Provide a way to intercept commands for testing Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:50   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 09/18] buildman: Set up bsettings outside the control module Simon Glass
                   ` (9 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

Buildman currently lacks testing in many areas, including its use of git,
make and many command-line flags.

Add a functional test which covers some of these areas. So far it does
a fake 'build' of all boards for the current source tree.

This version reads the real ~/.buildman and boards.cfg files. Future work
will improve this.

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

Changes in v3: None
Changes in v2:
- Add a comment to _HandleCommandGit
- Make sure the test temporary directory is removed

 tools/buildman/buildman.py  |  17 ++---
 tools/buildman/control.py   |  21 +++--
 tools/buildman/func_test.py | 182 ++++++++++++++++++++++++++++++++++++++++++++
 tools/buildman/toolchain.py |   4 +-
 4 files changed, 206 insertions(+), 18 deletions(-)
 create mode 100644 tools/buildman/func_test.py

diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py
index 70c2142..6771c86 100755
--- a/tools/buildman/buildman.py
+++ b/tools/buildman/buildman.py
@@ -30,27 +30,20 @@ import terminal
 import toolchain
 
 def RunTests():
+    import func_test
     import test
     import doctest
 
     result = unittest.TestResult()
-    for module in ['toolchain']:
+    for module in ['toolchain', 'gitutil']:
         suite = doctest.DocTestSuite(module)
         suite.run(result)
 
-    # TODO: Surely we can just 'print' result?
-    print result
-    for test, err in result.errors:
-        print err
-    for test, err in result.failures:
-        print err
-
     sys.argv = [sys.argv[0]]
-    suite = unittest.TestLoader().loadTestsFromTestCase(test.TestBuild)
-    result = unittest.TestResult()
-    suite.run(result)
+    for module in (test.TestBuild, func_test.TestFunctional):
+        suite = unittest.TestLoader().loadTestsFromTestCase(module)
+        suite.run(result)
 
-    # TODO: Surely we can just 'print' result?
     print result
     for test, err in result.errors:
         print err
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 408d9b1..213e235 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -13,6 +13,7 @@ from builder import Builder
 import gitutil
 import patchstream
 import terminal
+from terminal import Print
 import toolchain
 import command
 import subprocess
@@ -77,12 +78,18 @@ def ShowActions(series, why_selected, boards_selected, builder, options):
     print ('Total boards to build for each commit: %d\n' %
             why_selected['all'])
 
-def DoBuildman(options, args):
+def DoBuildman(options, args, toolchains=None, make_func=None):
     """The main control code for buildman
 
     Args:
         options: Command line options object
         args: Command line arguments (list of strings)
+        toolchains: Toolchains to use - this should be a Toolchains()
+                object. If None, then it will be created and scanned
+        make_func: Make function to use for the builder. This is called
+                to execute 'make'. If this is None, the normal function
+                will be used, which calls the 'make' tool with suitable
+                arguments. This setting is useful for tests.
     """
     if options.full_help:
         pager = os.getenv('PAGER')
@@ -97,8 +104,10 @@ def DoBuildman(options, args):
     bsettings.Setup(options.config_file)
     options.git_dir = os.path.join(options.git, '.git')
 
-    toolchains = toolchain.Toolchains()
-    toolchains.Scan(options.list_tool_chains)
+    if not toolchains:
+        toolchains = toolchain.Toolchains()
+        toolchains.GetSettings()
+        toolchains.Scan(options.list_tool_chains)
     if options.list_tool_chains:
         toolchains.List()
         print
@@ -202,6 +211,8 @@ def DoBuildman(options, args):
             options.threads, options.jobs, gnu_make=gnu_make, checkout=True,
             show_unknown=options.show_unknown, step=options.step)
     builder.force_config_on_failure = not options.quick
+    if make_func:
+        builder.do_make = make_func
 
     # For a dry run, just show our actions as a sanity check
     if options.dry_run:
@@ -220,8 +231,8 @@ def DoBuildman(options, args):
         else:
             commits = None
 
-        print GetActionSummary(options.summary, commits, board_selected,
-                               options)
+        Print(GetActionSummary(options.summary, commits, board_selected,
+                                options))
 
         builder.SetDisplayOptions(options.show_errors, options.show_sizes,
                                   options.show_detail, options.show_bloat,
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
new file mode 100644
index 0000000..8711f9c
--- /dev/null
+++ b/tools/buildman/func_test.py
@@ -0,0 +1,182 @@
+#
+# Copyright (c) 2014 Google, Inc
+#
+# SPDX-License-Identifier:      GPL-2.0+
+#
+
+import os
+import shutil
+import sys
+import tempfile
+import unittest
+
+import cmdline
+import command
+import control
+import gitutil
+import terminal
+import toolchain
+
+class TestFunctional(unittest.TestCase):
+    """Functional test for buildman.
+
+    This aims to test from just below the invocation of buildman (parsing
+    of arguments) to 'make' and 'git' invocation. It is not a true
+    emd-to-end test, as it mocks git, make and the tool chain. But this
+    makes it easier to detect when the builder is doing the wrong thing,
+    since in many cases this test code will fail. For example, only a
+    very limited subset of 'git' arguments is supported - anything
+    unexpected will fail.
+    """
+    def setUp(self):
+        self._base_dir = tempfile.mkdtemp()
+        self._git_dir = os.path.join(self._base_dir, 'src')
+        self._buildman_pathname = sys.argv[0]
+        self._buildman_dir = os.path.dirname(sys.argv[0])
+        command.test_result = self._HandleCommand
+        self._toolchains = toolchain.Toolchains()
+        self._toolchains.Add('gcc', test=False)
+
+    def tearDown(self):
+        shutil.rmtree(self._base_dir)
+
+    def _RunBuildman(self, *args):
+        return command.RunPipe([[self._buildman_pathname] + list(args)],
+                capture=True, capture_stderr=True)
+
+    def _RunControl(self, *args):
+        sys.argv = [sys.argv[0]] + list(args)
+        options, args = cmdline.ParseArgs()
+        return control.DoBuildman(options, args, toolchains=self._toolchains,
+                make_func=self._HandleMake)
+
+    def testFullHelp(self):
+        command.test_result = None
+        result = self._RunBuildman('-H')
+        help_file = os.path.join(self._buildman_dir, 'README')
+        self.assertEqual(len(result.stdout), os.path.getsize(help_file))
+        self.assertEqual(0, len(result.stderr))
+        self.assertEqual(0, result.return_code)
+
+    def testHelp(self):
+        command.test_result = None
+        result = self._RunBuildman('-h')
+        help_file = os.path.join(self._buildman_dir, 'README')
+        self.assertTrue(len(result.stdout) > 1000)
+        self.assertEqual(0, len(result.stderr))
+        self.assertEqual(0, result.return_code)
+
+    def testGitSetup(self):
+        """Test gitutils.Setup(), from outside the module itself"""
+        command.test_result = command.CommandResult(return_code=1)
+        gitutil.Setup()
+        self.assertEqual(gitutil.use_no_decorate, False)
+
+        command.test_result = command.CommandResult(return_code=0)
+        gitutil.Setup()
+        self.assertEqual(gitutil.use_no_decorate, True)
+
+    def _HandleCommandGitLog(self, args):
+        if '-n0' in args:
+            return command.CommandResult(return_code=0)
+
+        # Not handled, so abort
+        print 'git log', args
+        sys.exit(1)
+
+    def _HandleCommandGit(self, in_args):
+        """Handle execution of a git command
+
+        This uses a hacked-up parser.
+
+        Args:
+            in_args: Arguments after 'git' from the command line
+        """
+        git_args = []           # Top-level arguments to git itself
+        sub_cmd = None          # Git sub-command selected
+        args = []               # Arguments to the git sub-command
+        for arg in in_args:
+            if sub_cmd:
+                args.append(arg)
+            elif arg[0] == '-':
+                git_args.append(arg)
+            else:
+                sub_cmd = arg
+        if sub_cmd == 'config':
+            return command.CommandResult(return_code=0)
+        elif sub_cmd == 'log':
+            return self._HandleCommandGitLog(args)
+
+        # Not handled, so abort
+        print 'git', git_args, sub_cmd, args
+        sys.exit(1)
+
+    def _HandleCommandNm(self, args):
+        return command.CommandResult(return_code=0)
+
+    def _HandleCommandObjdump(self, args):
+        return command.CommandResult(return_code=0)
+
+    def _HandleCommandSize(self, args):
+        return command.CommandResult(return_code=0)
+
+    def _HandleCommand(self, **kwargs):
+        """Handle a command execution.
+
+        The command is in kwargs['pipe-list'], as a list of pipes, each a
+        list of commands. The command should be emulated as required for
+        testing purposes.
+
+        Returns:
+            A CommandResult object
+        """
+        pipe_list = kwargs['pipe_list']
+        if len(pipe_list) != 1:
+            print 'invalid pipe', kwargs
+            sys.exit(1)
+        cmd = pipe_list[0][0]
+        args = pipe_list[0][1:]
+        if cmd == 'git':
+            return self._HandleCommandGit(args)
+        elif cmd == './scripts/show-gnu-make':
+            return command.CommandResult(return_code=0, stdout='make')
+        elif cmd == 'nm':
+            return self._HandleCommandNm(args)
+        elif cmd == 'objdump':
+            return self._HandleCommandObjdump(args)
+        elif cmd == 'size':
+            return self._HandleCommandSize(args)
+
+        # Not handled, so abort
+        print 'unknown command', kwargs
+        sys.exit(1)
+        return command.CommandResult(return_code=0)
+
+    def _HandleMake(self, commit, brd, stage, cwd, *args, **kwargs):
+        """Handle execution of 'make'
+
+        Args:
+            commit: Commit object that is being built
+            brd: Board object that is being built
+            stage: Stage that we are at (mrproper, config, build)
+            cwd: Directory where make should be run
+            args: Arguments to pass to make
+            kwargs: Arguments to pass to command.RunPipe()
+        """
+        if stage == 'mrproper':
+            return command.CommandResult(return_code=0)
+        elif stage == 'config':
+            return command.CommandResult(return_code=0,
+                    combined='Test configuration complete')
+        elif stage == 'build':
+            return command.CommandResult(return_code=0)
+
+        # Not handled, so abort
+        print 'make', stage
+        sys.exit(1)
+
+    def testCurrentSource(self):
+        """Very simple test to invoke buildman on the current source"""
+        self._RunControl()
+        lines = terminal.GetPrintTestLines()
+        self.assertTrue(lines[0].text.startswith('Building current source'))
diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py
index 0e91294..27dc318 100644
--- a/tools/buildman/toolchain.py
+++ b/tools/buildman/toolchain.py
@@ -99,6 +99,9 @@ class Toolchains:
     def __init__(self):
         self.toolchains = {}
         self.paths = []
+        self._make_flags = dict(bsettings.GetItems('make-flags'))
+
+    def GetSettings(self):
         toolchains = bsettings.GetItems('toolchain')
         if not toolchains:
             print ("Warning: No tool chains - please add a [toolchain] section"
@@ -110,7 +113,6 @@ class Toolchains:
                 self.paths += glob.glob(value)
             else:
                 self.paths.append(value)
-        self._make_flags = dict(bsettings.GetItems('make-flags'))
 
     def Add(self, fname, test=True, verbose=False):
         """Add a toolchain to our list
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 09/18] buildman: Set up bsettings outside the control module
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (7 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 08/18] buildman: Add a functional test Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:50   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 10/18] buildman: Avoid looking at config file or toolchains in tests Simon Glass
                   ` (8 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

Move the bsettings code back to the main buildman.py file, so we can do
something different when testing.

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

Changes in v3: None
Changes in v2: None

 tools/buildman/buildman.py | 2 ++
 tools/buildman/control.py  | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/buildman/buildman.py b/tools/buildman/buildman.py
index 6771c86..d0afeda 100755
--- a/tools/buildman/buildman.py
+++ b/tools/buildman/buildman.py
@@ -19,6 +19,7 @@ sys.path.append(os.path.join(our_path, '../patman'))
 
 # Our modules
 import board
+import bsettings
 import builder
 import checkpatch
 import cmdline
@@ -59,5 +60,6 @@ if options.test:
 
 # Build selected commits for selected boards
 else:
+    bsettings.Setup(options.config_file)
     ret_code = control.DoBuildman(options, args)
     sys.exit(ret_code)
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 213e235..c8eeb6a 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -101,7 +101,6 @@ def DoBuildman(options, args, toolchains=None, make_func=None):
 
     gitutil.Setup()
 
-    bsettings.Setup(options.config_file)
     options.git_dir = os.path.join(options.git, '.git')
 
     if not toolchains:
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 10/18] buildman: Avoid looking at config file or toolchains in tests
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (8 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 09/18] buildman: Set up bsettings outside the control module Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:53   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 11/18] buildman: Allow tests to have their own boards Simon Glass
                   ` (7 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

These files may not exist in the environment, or may not be suitable for
testing. Provide our own config file and our own toolchains when running
tests.

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

Changes in v3: None
Changes in v2: None

 tools/buildman/bsettings.py | 15 ++++++++++-----
 tools/buildman/func_test.py | 19 +++++++++++++++++++
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py
index 9164798..fdd875b 100644
--- a/tools/buildman/bsettings.py
+++ b/tools/buildman/bsettings.py
@@ -5,6 +5,7 @@
 
 import ConfigParser
 import os
+import StringIO
 
 
 def Setup(fname=''):
@@ -17,11 +18,15 @@ def Setup(fname=''):
     global config_fname
 
     settings = ConfigParser.SafeConfigParser()
-    config_fname = fname
-    if config_fname == '':
-        config_fname = '%s/.buildman' % os.getenv('HOME')
-    if config_fname:
-        settings.read(config_fname)
+    if fname is not None:
+        config_fname = fname
+        if config_fname == '':
+            config_fname = '%s/.buildman' % os.getenv('HOME')
+        if config_fname:
+            settings.read(config_fname)
+
+def AddFile(data):
+    settings.readfp(StringIO.StringIO(data))
 
 def GetItems(section):
     """Get the items from a section of the config.
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 8711f9c..b92cde3 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -10,6 +10,7 @@ import sys
 import tempfile
 import unittest
 
+import bsettings
 import cmdline
 import command
 import control
@@ -17,6 +18,22 @@ import gitutil
 import terminal
 import toolchain
 
+settings_data = '''
+# Buildman settings file
+
+[toolchain]
+
+[toolchain-alias]
+
+[make-flags]
+src=/home/sjg/c/src
+chroot=/home/sjg/c/chroot
+vboot=USE_STDINT=1 VBOOT_DEBUG=1 MAKEFLAGS_VBOOT=DEBUG=1 CFLAGS_EXTRA_VBOOT=-DUNROLL_LOOPS VBOOT_SOURCE=${src}/platform/vboot_reference
+chromeos_coreboot=VBOOT=${chroot}/build/link/usr ${vboot}
+chromeos_daisy=VBOOT=${chroot}/build/daisy/usr ${vboot}
+chromeos_peach=VBOOT=${chroot}/build/peach_pit/usr ${vboot}
+'''
+
 class TestFunctional(unittest.TestCase):
     """Functional test for buildman.
 
@@ -36,6 +53,8 @@ class TestFunctional(unittest.TestCase):
         command.test_result = self._HandleCommand
         self._toolchains = toolchain.Toolchains()
         self._toolchains.Add('gcc', test=False)
+        bsettings.Setup(None)
+        bsettings.AddFile(settings_data)
 
     def tearDown(self):
         shutil.rmtree(self._base_dir)
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 11/18] buildman: Allow tests to have their own boards
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (9 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 10/18] buildman: Avoid looking at config file or toolchains in tests Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:53   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 12/18] buildman: Correct counting of build failures on retry Simon Glass
                   ` (6 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

Rather than reading boards.cfg, which may take time to generate and is not
necessarily suitable for running tests, create our own list of boards.

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

Changes in v3: None
Changes in v2: None

 tools/buildman/control.py   | 21 ++++++++++++---------
 tools/buildman/func_test.py | 20 +++++++++++++++++++-
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index c8eeb6a..fb15ae8 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -78,7 +78,7 @@ def ShowActions(series, why_selected, boards_selected, builder, options):
     print ('Total boards to build for each commit: %d\n' %
             why_selected['all'])
 
-def DoBuildman(options, args, toolchains=None, make_func=None):
+def DoBuildman(options, args, toolchains=None, make_func=None, boards=None):
     """The main control code for buildman
 
     Args:
@@ -90,6 +90,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None):
                 to execute 'make'. If this is None, the normal function
                 will be used, which calls the 'make' tool with suitable
                 arguments. This setting is useful for tests.
+        board: Boards() object to use, containing a list of available
+                boards. If this is None it will be created and scanned.
     """
     if options.full_help:
         pager = os.getenv('PAGER')
@@ -135,14 +137,15 @@ def DoBuildman(options, args, toolchains=None, make_func=None):
         sys.exit(col.Color(col.RED, str))
 
     # Work out what subset of the boards we are building
-    board_file = os.path.join(options.git, 'boards.cfg')
-    status = subprocess.call([os.path.join(options.git,
-                                           'tools/genboardscfg.py')])
-    if status != 0:
-        sys.exit("Failed to generate boards.cfg")
-
-    boards = board.Boards()
-    boards.ReadBoards(os.path.join(options.git, 'boards.cfg'))
+    if not boards:
+        board_file = os.path.join(options.git, 'boards.cfg')
+        status = subprocess.call([os.path.join(options.git,
+                                                'tools/genboardscfg.py')])
+        if status != 0:
+                sys.exit("Failed to generate boards.cfg")
+
+        boards = board.Boards()
+        boards.ReadBoards(os.path.join(options.git, 'boards.cfg'))
 
     exclude = []
     if options.exclude:
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index b92cde3..237a80b 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -10,6 +10,7 @@ import sys
 import tempfile
 import unittest
 
+import board
 import bsettings
 import cmdline
 import command
@@ -34,6 +35,14 @@ chromeos_daisy=VBOOT=${chroot}/build/daisy/usr ${vboot}
 chromeos_peach=VBOOT=${chroot}/build/peach_pit/usr ${vboot}
 '''
 
+boards = [
+    ['Active', 'arm', 'armv7', '', 'Tester', 'ARM Board 1', 'board0',  ''],
+    ['Active', 'arm', 'armv7', '', 'Tester', 'ARM Board 2', 'board1', ''],
+    ['Active', 'powerpc', 'powerpc', '', 'Tester', 'PowerPC board 1', 'board2', ''],
+    ['Active', 'powerpc', 'mpc5xx', '', 'Tester', 'PowerPC board 2', 'board3', ''],
+    ['Active', 'sandbox', 'sandbox', '', 'Tester', 'Sandbox board', 'board4', ''],
+]
+
 class TestFunctional(unittest.TestCase):
     """Functional test for buildman.
 
@@ -55,6 +64,9 @@ class TestFunctional(unittest.TestCase):
         self._toolchains.Add('gcc', test=False)
         bsettings.Setup(None)
         bsettings.AddFile(settings_data)
+        self._boards = board.Boards()
+        for brd in boards:
+            self._boards.AddBoard(board.Board(*brd))
 
     def tearDown(self):
         shutil.rmtree(self._base_dir)
@@ -67,7 +79,7 @@ class TestFunctional(unittest.TestCase):
         sys.argv = [sys.argv[0]] + list(args)
         options, args = cmdline.ParseArgs()
         return control.DoBuildman(options, args, toolchains=self._toolchains,
-                make_func=self._HandleMake)
+                make_func=self._HandleMake, boards=self._boards)
 
     def testFullHelp(self):
         command.test_result = None
@@ -194,6 +206,12 @@ class TestFunctional(unittest.TestCase):
         print 'make', stage
         sys.exit(1)
 
+    def testNoBoards(self):
+        """Test that buildman aborts when there are no boards"""
+        self._boards = board.Boards()
+        with self.assertRaises(SystemExit):
+            self._RunControl()
+
     def testCurrentSource(self):
         """Very simple test to invoke buildman on the current source"""
         self._RunControl()
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 12/18] buildman: Correct counting of build failures on retry
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (10 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 11/18] buildman: Allow tests to have their own boards Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:53   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 13/18] buildman: Provide an internal option to clean the outpur dir Simon Glass
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

When a build is to be performed, buildman checks to see if it has already
been done. In most cases it will not bother trying again. However, it was
not reading the return code from the 'done' file, so if the result was a
failure, it would not be counted. This depresses the 'failure' count stats
that buildman prints in this case.

Fix this bug by always reading the return code.

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

Changes in v3: None
Changes in v2: None

 tools/buildman/builderthread.py | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 0246375..261919f 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -138,16 +138,17 @@ class BuilderThread(threading.Thread):
         result.already_done = os.path.exists(done_file)
         will_build = (force_build or force_build_failures or
             not result.already_done)
-        if result.already_done and will_build:
+        if result.already_done:
             # Get the return code from that build and use it
             with open(done_file, 'r') as fd:
                 result.return_code = int(fd.readline())
-            err_file = self.builder.GetErrFile(commit_upto, brd.target)
-            if os.path.exists(err_file) and os.stat(err_file).st_size:
-                result.stderr = 'bad'
-            elif not force_build:
-                # The build passed, so no need to build it again
-                will_build = False
+            if will_build:
+                err_file = self.builder.GetErrFile(commit_upto, brd.target)
+                if os.path.exists(err_file) and os.stat(err_file).st_size:
+                    result.stderr = 'bad'
+                elif not force_build:
+                    # The build passed, so no need to build it again
+                    will_build = False
 
         if will_build:
             # We are going to have to build it. First, get a toolchain
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 13/18] buildman: Provide an internal option to clean the outpur dir
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (11 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 12/18] buildman: Correct counting of build failures on retry Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:53   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 14/18] patman: Start with a clean series when needed Simon Glass
                   ` (4 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

For testing it is useful to clean the output directory before running a
test. This avoids a test interfering with the results of a subsequent
test by leaving data around.

Add this feature as an optional parameter to the control logic.

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

Changes in v3: None
Changes in v2: None

 tools/buildman/control.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index fb15ae8..2f51249 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -5,6 +5,7 @@
 
 import multiprocessing
 import os
+import shutil
 import sys
 
 import board
@@ -78,7 +79,8 @@ def ShowActions(series, why_selected, boards_selected, builder, options):
     print ('Total boards to build for each commit: %d\n' %
             why_selected['all'])
 
-def DoBuildman(options, args, toolchains=None, make_func=None, boards=None):
+def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
+               clean_dir=False):
     """The main control code for buildman
 
     Args:
@@ -93,6 +95,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None):
         board: Boards() object to use, containing a list of available
                 boards. If this is None it will be created and scanned.
     """
+    global builder
+
     if options.full_help:
         pager = os.getenv('PAGER')
         if not pager:
@@ -209,6 +213,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None):
     else:
         dirname = 'current'
     output_dir = os.path.join(options.output_dir, dirname)
+    if clean_dir:
+        shutil.rmtree(output_dir)
     builder = Builder(toolchains, output_dir, options.git_dir,
             options.threads, options.jobs, gnu_make=gnu_make, checkout=True,
             show_unknown=options.show_unknown, step=options.step)
@@ -230,6 +236,9 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None):
 
         if series:
             commits = series.commits
+            # Number the commits for test purposes
+            for commit in range(len(commits)):
+                commits[commit].sequence = commit
         else:
             commits = None
 
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 14/18] patman: Start with a clean series when needed
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (12 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 13/18] buildman: Provide an internal option to clean the outpur dir Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:54   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 15/18] buildman: Add additional functional tests Simon Glass
                   ` (3 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

For reasons that are not well-understood, GetMetaDataForList() can end up
adding to an existing series even when it appears that it should be
starting a new one.

Change from using a default constructor parameter to an explicit one, to
work around this problem.

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

Changes in v3: None
Changes in v2: None

 tools/patman/patchstream.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index b0b8153..b3e66c3 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -355,7 +355,7 @@ class PatchStream:
 
 
 def GetMetaDataForList(commit_range, git_dir=None, count=None,
-                       series = Series()):
+                       series = None):
     """Reads out patch series metadata from the commits
 
     This does a 'git log' on the relevant commits and pulls out the tags we
@@ -370,6 +370,8 @@ def GetMetaDataForList(commit_range, git_dir=None, count=None,
     Returns:
         A Series object containing information about the commits.
     """
+    if not series:
+        series = Series()
     params = gitutil.LogCmd(commit_range,reverse=True, count=count,
                             git_dir=git_dir)
     stdout = command.RunPipe([params], capture=True).stdout
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 15/18] buildman: Add additional functional tests
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (13 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 14/18] patman: Start with a clean series when needed Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:54   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 16/18] buildman: Expand output test to cover directory prefixes Simon Glass
                   ` (2 subsequent siblings)
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

This adds coverage of core features of the builder, including the
command-line options which affect building.

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

Changes in v3: None
Changes in v2: None

 tools/buildman/func_test.py | 324 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 306 insertions(+), 18 deletions(-)

diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 237a80b..2cb5cf0 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -43,6 +43,125 @@ boards = [
     ['Active', 'sandbox', 'sandbox', '', 'Tester', 'Sandbox board', 'board4', ''],
 ]
 
+commit_shortlog = """4aca821 patman: Avoid changing the order of tags
+39403bb patman: Use --no-pager' to stop git from forking a pager
+db6e6f2 patman: Remove the -a option
+f2ccf03 patman: Correct unit tests to run correctly
+1d097f9 patman: Fix indentation in terminal.py
+d073747 patman: Support the 'reverse' option for 'git log
+"""
+
+commit_log = ["""commit 7f6b8315d18f683c5181d0c3694818c1b2a20dcd
+Author: Masahiro Yamada <yamada.m@jp.panasonic.com>
+Date:   Fri Aug 22 19:12:41 2014 +0900
+
+    buildman: refactor help message
+
+    "buildman [options]" is displayed by default.
+
+    Append the rest of help messages to parser.usage
+    instead of replacing it.
+
+    Besides, "-b <branch>" is not mandatory since commit fea5858e.
+    Drop it from the usage.
+
+    Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
+""",
+"""commit d0737479be6baf4db5e2cdbee123e96bc5ed0ba8
+Author: Simon Glass <sjg@chromium.org>
+Date:   Thu Aug 14 16:48:25 2014 -0600
+
+    patman: Support the 'reverse' option for 'git log'
+
+    This option is currently not supported, but needs to be, for buildman to
+    operate as expected.
+
+    Series-changes: 7
+    - Add new patch to fix the 'reverse' bug
+
+
+    Change-Id: I79078f792e8b390b8a1272a8023537821d45feda
+    Reported-by: York Sun <yorksun@freescale.com>
+    Signed-off-by: Simon Glass <sjg@chromium.org>
+
+""",
+"""commit 1d097f9ab487c5019152fd47bda126839f3bf9fc
+Author: Simon Glass <sjg@chromium.org>
+Date:   Sat Aug 9 11:44:32 2014 -0600
+
+    patman: Fix indentation in terminal.py
+
+    This code came from a different project with 2-character indentation. Fix
+    it for U-Boot.
+
+    Series-changes: 6
+    - Add new patch to fix indentation in teminal.py
+
+    Change-Id: I5a74d2ebbb3cc12a665f5c725064009ac96e8a34
+    Signed-off-by: Simon Glass <sjg@chromium.org>
+
+""",
+"""commit f2ccf03869d1e152c836515a3ceb83cdfe04a105
+Author: Simon Glass <sjg@chromium.org>
+Date:   Sat Aug 9 11:08:24 2014 -0600
+
+    patman: Correct unit tests to run correctly
+
+    It seems that doctest behaves differently now, and some of the unit tests
+    do not run. Adjust the tests to work correctly.
+
+     ./tools/patman/patman --test
+    <unittest.result.TestResult run=10 errors=0 failures=0>
+
+    Series-changes: 6
+    - Add new patch to fix patman unit tests
+
+    Change-Id: I3d2ca588f4933e1f9d6b1665a00e4ae58269ff3b
+
+""",
+"""commit db6e6f2f9331c5a37647d6668768d4a40b8b0d1c
+Author: Simon Glass <sjg@chromium.org>
+Date:   Sat Aug 9 12:06:02 2014 -0600
+
+    patman: Remove the -a option
+
+    It seems that this is no longer needed, since checkpatch.pl will catch
+    whitespace problems in patches. Also the option is not widely used, so
+    it seems safe to just remove it.
+
+    Series-changes: 6
+    - Add new patch to remove patman's -a option
+
+    Suggested-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
+    Change-Id: I5821a1c75154e532c46513486ca40b808de7e2cc
+
+""",
+"""commit 39403bb4f838153028a6f21ca30bf100f3791133
+Author: Simon Glass <sjg@chromium.org>
+Date:   Thu Aug 14 21:50:52 2014 -0600
+
+    patman: Use --no-pager' to stop git from forking a pager
+
+""",
+"""commit 4aca821e27e97925c039e69fd37375b09c6f129c
+Author: Simon Glass <sjg@chromium.org>
+Date:   Fri Aug 22 15:57:39 2014 -0600
+
+    patman: Avoid changing the order of tags
+
+    patman collects tags that it sees in the commit and places them nicely
+    sorted at the end of the patch. However, this is not really necessary and
+    in fact is apparently not desirable.
+
+    Series-changes: 9
+    - Add new patch to avoid changing the order of tags
+
+    Suggested-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
+    Change-Id: Ib1518588c1a189ad5c3198aae76f8654aed8d0db
+"""]
+
+TEST_BRANCH = '__testbranch'
+
 class TestFunctional(unittest.TestCase):
     """Functional test for buildman.
 
@@ -60,26 +179,49 @@ class TestFunctional(unittest.TestCase):
         self._buildman_pathname = sys.argv[0]
         self._buildman_dir = os.path.dirname(sys.argv[0])
         command.test_result = self._HandleCommand
-        self._toolchains = toolchain.Toolchains()
-        self._toolchains.Add('gcc', test=False)
+        self.setupToolchains()
+        self._toolchains.Add('arm-gcc', test=False)
+        self._toolchains.Add('powerpc-gcc', test=False)
         bsettings.Setup(None)
         bsettings.AddFile(settings_data)
         self._boards = board.Boards()
         for brd in boards:
             self._boards.AddBoard(board.Board(*brd))
 
+        # Directories where the source been cloned
+        self._clone_dirs = []
+        self._commits = len(commit_shortlog.splitlines()) + 1
+        self._total_builds = self._commits * len(boards)
+
+        # Number of calls to make
+        self._make_calls = 0
+
+        # Map of [board, commit] to error messages
+        self._error = {}
+
+        # Avoid sending any output and clear all terminal output
+        terminal.SetPrintTestMode()
+        terminal.GetPrintTestLines()
+
     def tearDown(self):
         shutil.rmtree(self._base_dir)
 
+    def setupToolchains(self):
+        self._toolchains = toolchain.Toolchains()
+        self._toolchains.Add('gcc', test=False)
+
     def _RunBuildman(self, *args):
         return command.RunPipe([[self._buildman_pathname] + list(args)],
                 capture=True, capture_stderr=True)
 
-    def _RunControl(self, *args):
+    def _RunControl(self, *args, **kwargs):
         sys.argv = [sys.argv[0]] + list(args)
         options, args = cmdline.ParseArgs()
-        return control.DoBuildman(options, args, toolchains=self._toolchains,
-                make_func=self._HandleMake, boards=self._boards)
+        result = control.DoBuildman(options, args, toolchains=self._toolchains,
+                make_func=self._HandleMake, boards=self._boards,
+                clean_dir=kwargs.get('clean_dir', True))
+        self._builder = control.builder
+        return result
 
     def testFullHelp(self):
         command.test_result = None
@@ -110,11 +252,34 @@ class TestFunctional(unittest.TestCase):
     def _HandleCommandGitLog(self, args):
         if '-n0' in args:
             return command.CommandResult(return_code=0)
+        elif args[-1] == 'upstream/master..%s' % TEST_BRANCH:
+            return command.CommandResult(return_code=0, stdout=commit_shortlog)
+        elif args[:3] == ['--no-color', '--no-decorate', '--reverse']:
+            if args[-1] == TEST_BRANCH:
+                count = int(args[3][2:])
+                return command.CommandResult(return_code=0,
+                                            stdout=''.join(commit_log[:count]))
 
         # Not handled, so abort
         print 'git log', args
         sys.exit(1)
 
+    def _HandleCommandGitConfig(self, args):
+        config = args[0]
+        if config == 'sendemail.aliasesfile':
+            return command.CommandResult(return_code=0)
+        elif config.startswith('branch.badbranch'):
+            return command.CommandResult(return_code=1)
+        elif config == 'branch.%s.remote' % TEST_BRANCH:
+            return command.CommandResult(return_code=0, stdout='upstream\n')
+        elif config == 'branch.%s.merge' % TEST_BRANCH:
+            return command.CommandResult(return_code=0,
+                                         stdout='refs/heads/master\n')
+
+        # Not handled, so abort
+        print 'git config', args
+        sys.exit(1)
+
     def _HandleCommandGit(self, in_args):
         """Handle execution of a git command
 
@@ -132,11 +297,18 @@ class TestFunctional(unittest.TestCase):
             elif arg[0] == '-':
                 git_args.append(arg)
             else:
-                sub_cmd = arg
+                if git_args and git_args[-1] in ['--git-dir', '--work-tree']:
+                    git_args.append(arg)
+                else:
+                    sub_cmd = arg
         if sub_cmd == 'config':
-            return command.CommandResult(return_code=0)
+            return self._HandleCommandGitConfig(args)
         elif sub_cmd == 'log':
             return self._HandleCommandGitLog(args)
+        elif sub_cmd == 'clone':
+            return command.CommandResult(return_code=0)
+        elif sub_cmd == 'checkout':
+            return command.CommandResult(return_code=0)
 
         # Not handled, so abort
         print 'git', git_args, sub_cmd, args
@@ -162,26 +334,35 @@ class TestFunctional(unittest.TestCase):
             A CommandResult object
         """
         pipe_list = kwargs['pipe_list']
+        wc = False
         if len(pipe_list) != 1:
-            print 'invalid pipe', kwargs
-            sys.exit(1)
+            if pipe_list[1] == ['wc', '-l']:
+                wc = True
+            else:
+                print 'invalid pipe', kwargs
+                sys.exit(1)
         cmd = pipe_list[0][0]
         args = pipe_list[0][1:]
+        result = None
         if cmd == 'git':
-            return self._HandleCommandGit(args)
+            result = self._HandleCommandGit(args)
         elif cmd == './scripts/show-gnu-make':
             return command.CommandResult(return_code=0, stdout='make')
-        elif cmd == 'nm':
+        elif cmd.endswith('nm'):
             return self._HandleCommandNm(args)
-        elif cmd == 'objdump':
+        elif cmd.endswith('objdump'):
             return self._HandleCommandObjdump(args)
-        elif cmd == 'size':
+        elif cmd.endswith( 'size'):
             return self._HandleCommandSize(args)
 
-        # Not handled, so abort
-        print 'unknown command', kwargs
-        sys.exit(1)
-        return command.CommandResult(return_code=0)
+        if not result:
+            # Not handled, so abort
+            print 'unknown command', kwargs
+            sys.exit(1)
+
+        if wc:
+            result.stdout = len(result.stdout.splitlines())
+        return result
 
     def _HandleMake(self, commit, brd, stage, cwd, *args, **kwargs):
         """Handle execution of 'make'
@@ -194,18 +375,31 @@ class TestFunctional(unittest.TestCase):
             args: Arguments to pass to make
             kwargs: Arguments to pass to command.RunPipe()
         """
+        self._make_calls += 1
         if stage == 'mrproper':
             return command.CommandResult(return_code=0)
         elif stage == 'config':
             return command.CommandResult(return_code=0,
                     combined='Test configuration complete')
         elif stage == 'build':
+            stderr = ''
+            if type(commit) is not str:
+                stderr = self._error.get((brd.target, commit.sequence))
+            if stderr:
+                return command.CommandResult(return_code=1, stderr=stderr)
             return command.CommandResult(return_code=0)
 
         # Not handled, so abort
         print 'make', stage
         sys.exit(1)
 
+    # Example function to print output lines
+    def print_lines(self, lines):
+        print len(lines)
+        for line in lines:
+            print line
+        #self.print_lines(terminal.GetPrintTestLines())
+
     def testNoBoards(self):
         """Test that buildman aborts when there are no boards"""
         self._boards = board.Boards()
@@ -214,6 +408,100 @@ class TestFunctional(unittest.TestCase):
 
     def testCurrentSource(self):
         """Very simple test to invoke buildman on the current source"""
+        self.setupToolchains();
         self._RunControl()
         lines = terminal.GetPrintTestLines()
-        self.assertTrue(lines[0].text.startswith('Building current source'))
+        self.assertIn('Building current source for %d boards' % len(boards),
+                      lines[0].text)
+
+    def testBadBranch(self):
+        """Test that we can detect an invalid branch"""
+        with self.assertRaises(ValueError):
+            self._RunControl('-b', 'badbranch')
+
+    def testBadToolchain(self):
+        """Test that missing toolchains are detected"""
+        self.setupToolchains();
+        ret_code = self._RunControl('-b', TEST_BRANCH)
+        lines = terminal.GetPrintTestLines()
+
+        # Buildman always builds the upstream commit as well
+        self.assertIn('Building %d commits for %d boards' %
+                (self._commits, len(boards)), lines[0].text)
+        self.assertEqual(self._builder.count, self._total_builds)
+
+        # Only sandbox should succeed, the others don't have toolchains
+        self.assertEqual(self._builder.fail,
+                         self._total_builds - self._commits)
+        self.assertEqual(ret_code, 128)
+
+        for commit in range(self._commits):
+            for board in self._boards.GetList():
+                if board.arch != 'sandbox':
+                  errfile = self._builder.GetErrFile(commit, board.target)
+                  fd = open(errfile)
+                  self.assertEqual(fd.readlines(),
+                          ['No tool chain for %s\n' % board.arch])
+                  fd.close()
+
+    def testBranch(self):
+        """Test building a branch with all toolchains present"""
+        self._RunControl('-b', TEST_BRANCH)
+        self.assertEqual(self._builder.count, self._total_builds)
+        self.assertEqual(self._builder.fail, 0)
+
+    def testCount(self):
+        """Test building a specific number of commitst"""
+        self._RunControl('-b', TEST_BRANCH, '-c2')
+        self.assertEqual(self._builder.count, 2 * len(boards))
+        self.assertEqual(self._builder.fail, 0)
+        # Each board has a mrproper, config, and then one make per commit
+        self.assertEqual(self._make_calls, len(boards) * (2 + 2))
+
+    def testIncremental(self):
+        """Test building a branch twice - the second time should do nothing"""
+        self._RunControl('-b', TEST_BRANCH)
+
+        # Each board has a mrproper, config, and then one make per commit
+        self.assertEqual(self._make_calls, len(boards) * (self._commits + 2))
+        self._make_calls = 0
+        self._RunControl('-b', TEST_BRANCH, clean_dir=False)
+        self.assertEqual(self._make_calls, 0)
+        self.assertEqual(self._builder.count, self._total_builds)
+        self.assertEqual(self._builder.fail, 0)
+
+    def testForceBuild(self):
+        """The -f flag should force a rebuild"""
+        self._RunControl('-b', TEST_BRANCH)
+        self._make_calls = 0
+        self._RunControl('-b', TEST_BRANCH, '-f', clean_dir=False)
+        # Each board has a mrproper, config, and then one make per commit
+        self.assertEqual(self._make_calls, len(boards) * (self._commits + 2))
+
+    def testForceReconfigure(self):
+        """The -f flag should force a rebuild"""
+        self._RunControl('-b', TEST_BRANCH, '-C')
+        # Each commit has a mrproper, config and make
+        self.assertEqual(self._make_calls, len(boards) * self._commits * 3)
+
+    def testErrors(self):
+        """Test handling of build errors"""
+        self._error['board2', 1] = 'fred\n'
+        self._RunControl('-b', TEST_BRANCH)
+        self.assertEqual(self._builder.count, self._total_builds)
+        self.assertEqual(self._builder.fail, 1)
+
+        # Remove the error. This should have no effect since the commit will
+        # not be rebuilt
+        del self._error['board2', 1]
+        self._make_calls = 0
+        self._RunControl('-b', TEST_BRANCH, clean_dir=False)
+        self.assertEqual(self._builder.count, self._total_builds)
+        self.assertEqual(self._make_calls, 0)
+        self.assertEqual(self._builder.fail, 1)
+
+        # Now use the -F flag to force rebuild of the bad commit
+        self._RunControl('-b', TEST_BRANCH, '-F', clean_dir=False)
+        self.assertEqual(self._builder.count, self._total_builds)
+        self.assertEqual(self._builder.fail, 0)
+        self.assertEqual(self._make_calls, 3)
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 16/18] buildman: Expand output test to cover directory prefixes
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (14 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 15/18] buildman: Add additional functional tests Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:54   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 17/18] buildman: Permit branch names with an embedded '/' Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 18/18] buildman: Ignore conflicting tags Simon Glass
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

Now that buildman supports removing the build directory prefix from output,
add a test for it. Also ensure that output directories are removed when the
test completes.

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

Changes in v3: None
Changes in v2:
- Add patch to expand output test to cover directory prefixes

 tools/buildman/test.py | 54 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 8 deletions(-)

diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index f0c4d0e..a2a85ac 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -46,6 +46,20 @@ powerpc-linux-ld: u-boot: section .reloc lma 0xffffa400 overlaps previous sectio
 powerpc-linux-ld: u-boot: section .data lma 0xffffcd38 overlaps previous sections
 powerpc-linux-ld: u-boot: section .u_boot_cmd lma 0xffffeb40 overlaps previous sections
 powerpc-linux-ld: u-boot: section .bootpg lma 0xfffff198 overlaps previous sections
+''',
+   '''In file included from %(basedir)sarch/sandbox/cpu/cpu.c:9:0:
+%(basedir)sarch/sandbox/include/asm/state.h:44:0: warning: "xxxx" redefined [enabled by default]
+%(basedir)sarch/sandbox/include/asm/state.h:43:0: note: this is the location of the previous definition
+%(basedir)sarch/sandbox/cpu/cpu.c: In function 'do_reset':
+%(basedir)sarch/sandbox/cpu/cpu.c:27:1: error: unknown type name 'blah'
+%(basedir)sarch/sandbox/cpu/cpu.c:28:12: error: expected declaration specifiers or '...' before numeric constant
+make[2]: *** [arch/sandbox/cpu/cpu.o] Error 1
+make[1]: *** [arch/sandbox/cpu] Error 2
+make[1]: *** Waiting for unfinished jobs....
+In file included from %(basedir)scommon/board_f.c:55:0:
+%(basedir)sarch/sandbox/include/asm/state.h:44:0: warning: "xxxx" redefined [enabled by default]
+%(basedir)sarch/sandbox/include/asm/state.h:43:0: note: this is the location of the previous definition
+make: *** [sub-make] Error 2
 '''
 ]
 
@@ -57,7 +71,8 @@ commits = [
     ['9012', 'Third commit, error', 1, errors[0:2]],
     ['3456', 'Fourth commit, warning', 0, [errors[0], errors[2]]],
     ['7890', 'Fifth commit, link errors', 1, [errors[0], errors[3]]],
-    ['abcd', 'Sixth commit, fixes all errors', 0, []]
+    ['abcd', 'Sixth commit, fixes all errors', 0, []],
+    ['ef01', 'Seventh commit, check directory suppression', 1, [errors[4]]],
 ]
 
 boards = [
@@ -109,15 +124,19 @@ class TestBuild(unittest.TestCase):
         self._col = terminal.Color()
 
     def Make(self, commit, brd, stage, *args, **kwargs):
+        global base_dir
+
         result = command.CommandResult()
         boardnum = int(brd.target[-1])
         result.return_code = 0
         result.stderr = ''
         result.stdout = ('This is the test output for board %s, commit %s' %
                 (brd.target, commit.hash))
-        if boardnum >= 1 and boardnum >= commit.sequence:
+        if ((boardnum >= 1 and boardnum >= commit.sequence) or
+                boardnum == 4 and commit.sequence == 6):
             result.return_code = commit.return_code
-            result.stderr = ''.join(commit.error_list)
+            result.stderr = (''.join(commit.error_list)
+                % {'basedir' : base_dir + '/.bm-work/00/'})
         if stage == 'build':
             target_dir = None
             for arg in args:
@@ -146,10 +165,12 @@ class TestBuild(unittest.TestCase):
 
         This does a line-by-line verification of the summary output.
         """
-        output_dir = tempfile.mkdtemp()
-        if not os.path.isdir(output_dir):
-            os.mkdir(output_dir)
-        build = builder.Builder(self.toolchains, output_dir, None, 1, 2,
+        global base_dir
+
+        base_dir = tempfile.mkdtemp()
+        if not os.path.isdir(base_dir):
+            os.mkdir(base_dir)
+        build = builder.Builder(self.toolchains, base_dir, None, 1, 2,
                                 checkout=False, show_unknown=False)
         build.do_make = self.Make
         board_selected = self.boards.GetSelectedDict()
@@ -167,6 +188,7 @@ class TestBuild(unittest.TestCase):
         self.assertEqual(count, len(commits) * len(boards) + 1)
         build.SetDisplayOptions(show_errors=True);
         build.ShowSummary(self.commits, board_selected)
+        #terminal.EchoPrintTestLines()
         lines = terminal.GetPrintTestLines()
         self.assertEqual(lines[0].text, '01: %s' % commits[0][1])
         self.assertEqual(lines[1].text, '02: %s' % commits[1][1])
@@ -230,7 +252,22 @@ class TestBuild(unittest.TestCase):
         self.assertEqual(lines[24].text, 'w-%s' %
                 errors[0].rstrip().replace('\n', '\nw-'))
 
-        self.assertEqual(len(lines), 25)
+        self.assertEqual(lines[25].text, '07: %s' % commits[6][1])
+        self.assertSummary(lines[26].text, 'sandbox', '+', ['board4'])
+
+        # Pick out the correct error lines
+        expect_str = errors[4].rstrip().replace('%(basedir)s', '').split('\n')
+        expect = expect_str[3:8] + [expect_str[-1]]
+        self.assertEqual(lines[27].text, '+%s' %
+                '\n'.join(expect).replace('\n', '\n+'))
+
+        # Now the warnings lines
+        expect = [expect_str[0]] + expect_str[10:12] + [expect_str[9]]
+        self.assertEqual(lines[28].text, 'w+%s' %
+                '\n'.join(expect).replace('\n', '\nw+'))
+
+        self.assertEqual(len(lines), 29)
+        shutil.rmtree(base_dir)
 
     def _testGit(self):
         """Test basic builder operation by building a branch"""
@@ -255,6 +292,7 @@ class TestBuild(unittest.TestCase):
         options.keep_outputs = False
         args = ['tegra20']
         control.DoBuildman(options, args)
+        shutil.rmtree(base_dir)
 
     def testBoardSingle(self):
         """Test single board selection"""
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 17/18] buildman: Permit branch names with an embedded '/'
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (15 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 16/18] buildman: Expand output test to cover directory prefixes Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:54   ` Simon Glass
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 18/18] buildman: Ignore conflicting tags Simon Glass
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

At present buildman naively uses the branch name as part of its directory
path, which causes problems if the name has an embedded '/'.

Replace these with '_' to fix the problem.

Reported-by: Steve Rae <srae@broadcom.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Add new patch to permit branch names with an embedded '/'

Changes in v2: None

 tools/buildman/control.py   |  2 +-
 tools/buildman/func_test.py | 17 +++++++++++++----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 2f51249..06ca606 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -209,7 +209,7 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
 
     # Create a new builder with the selected options
     if options.branch:
-        dirname = options.branch
+        dirname = options.branch.replace('/', '_')
     else:
         dirname = 'current'
     output_dir = os.path.join(options.output_dir, dirname)
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 2cb5cf0..c37f1b6 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -199,6 +199,8 @@ class TestFunctional(unittest.TestCase):
         # Map of [board, commit] to error messages
         self._error = {}
 
+        self._test_branch = TEST_BRANCH
+
         # Avoid sending any output and clear all terminal output
         terminal.SetPrintTestMode()
         terminal.GetPrintTestLines()
@@ -252,10 +254,10 @@ class TestFunctional(unittest.TestCase):
     def _HandleCommandGitLog(self, args):
         if '-n0' in args:
             return command.CommandResult(return_code=0)
-        elif args[-1] == 'upstream/master..%s' % TEST_BRANCH:
+        elif args[-1] == 'upstream/master..%s' % self._test_branch:
             return command.CommandResult(return_code=0, stdout=commit_shortlog)
         elif args[:3] == ['--no-color', '--no-decorate', '--reverse']:
-            if args[-1] == TEST_BRANCH:
+            if args[-1] == self._test_branch:
                 count = int(args[3][2:])
                 return command.CommandResult(return_code=0,
                                             stdout=''.join(commit_log[:count]))
@@ -270,9 +272,9 @@ class TestFunctional(unittest.TestCase):
             return command.CommandResult(return_code=0)
         elif config.startswith('branch.badbranch'):
             return command.CommandResult(return_code=1)
-        elif config == 'branch.%s.remote' % TEST_BRANCH:
+        elif config == 'branch.%s.remote' % self._test_branch:
             return command.CommandResult(return_code=0, stdout='upstream\n')
-        elif config == 'branch.%s.merge' % TEST_BRANCH:
+        elif config == 'branch.%s.merge' % self._test_branch:
             return command.CommandResult(return_code=0,
                                          stdout='refs/heads/master\n')
 
@@ -505,3 +507,10 @@ class TestFunctional(unittest.TestCase):
         self.assertEqual(self._builder.count, self._total_builds)
         self.assertEqual(self._builder.fail, 0)
         self.assertEqual(self._make_calls, 3)
+
+    def testBranchWithSlash(self):
+        """Test building a branch with a '/' in the name"""
+        self._test_branch = '/__dev/__testbranch'
+        self._RunControl('-b', self._test_branch, clean_dir=False)
+        self.assertEqual(self._builder.count, self._total_builds)
+        self.assertEqual(self._builder.fail, 0)
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 18/18] buildman: Ignore conflicting tags
  2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
                   ` (16 preceding siblings ...)
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 17/18] buildman: Permit branch names with an embedded '/' Simon Glass
@ 2014-09-06  1:00 ` Simon Glass
  2014-09-10 18:55   ` Simon Glass
  17 siblings, 1 reply; 37+ messages in thread
From: Simon Glass @ 2014-09-06  1:00 UTC (permalink / raw)
  To: u-boot

Tags like Series-version are normally expected to appear once, and with a
unique value. But buildman doesn't actually look at these tags. So ignore
conflicts.

This allows bulidman to build a branch containing multiple patman series.

Reported-by: Steve Rae <srae@broadcom.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Add new patch to ignore conflicting tags in buildman

Changes in v2: None

 tools/buildman/control.py   | 15 +++++++--------
 tools/buildman/func_test.py |  3 +++
 tools/patman/patchstream.py |  4 +++-
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 06ca606..a2259c3 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -166,6 +166,10 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
     # upstream/master~..branch but that isn't possible if upstream/master is
     # a merge commit (it will list all the commits that form part of the
     # merge)
+    # Conflicting tags are not a problem for buildman, since it does not use
+    # them. For example, Series-version is not useful for buildman. On the
+    # other hand conflicting tags will cause an error. So allow later tags
+    # to overwrite earlier ones by setting allow_overwrite=True
     if options.branch:
         if count == -1:
             range_expr = gitutil.GetRangeInBranch(options.git_dir,
@@ -173,19 +177,14 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
             upstream_commit = gitutil.GetUpstream(options.git_dir,
                                                   options.branch)
             series = patchstream.GetMetaDataForList(upstream_commit,
-                options.git_dir, 1)
+                options.git_dir, 1, series=None, allow_overwrite=True)
 
-            # Conflicting tags are not a problem for buildman, since it does
-            # not use them. For example, Series-version is not useful for
-            # buildman. On the other hand conflicting tags will cause an
-            # error. So allow later tags to overwrite earlier ones.
-            series.allow_overwrite = True
             series = patchstream.GetMetaDataForList(range_expr,
-                                              options.git_dir, None, series)
+                    options.git_dir, None, series, allow_overwrite=True)
         else:
             # Honour the count
             series = patchstream.GetMetaDataForList(options.branch,
-                                                    options.git_dir, count)
+                    options.git_dir, count, series=None, allow_overwrite=True)
     else:
         series = None
         options.verbose = True
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index c37f1b6..75eb3a9 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -79,6 +79,7 @@ Date:   Thu Aug 14 16:48:25 2014 -0600
     Series-changes: 7
     - Add new patch to fix the 'reverse' bug
 
+    Series-version: 8
 
     Change-Id: I79078f792e8b390b8a1272a8023537821d45feda
     Reported-by: York Sun <yorksun@freescale.com>
@@ -156,6 +157,8 @@ Date:   Fri Aug 22 15:57:39 2014 -0600
     Series-changes: 9
     - Add new patch to avoid changing the order of tags
 
+    Series-version: 9
+
     Suggested-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
     Change-Id: Ib1518588c1a189ad5c3198aae76f8654aed8d0db
 """]
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index b3e66c3..d630157 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -355,7 +355,7 @@ class PatchStream:
 
 
 def GetMetaDataForList(commit_range, git_dir=None, count=None,
-                       series = None):
+                       series = None, allow_overwrite=False):
     """Reads out patch series metadata from the commits
 
     This does a 'git log' on the relevant commits and pulls out the tags we
@@ -367,11 +367,13 @@ def GetMetaDataForList(commit_range, git_dir=None, count=None,
         count: Number of commits to list, or None for no limit
         series: Series object to add information into. By default a new series
             is started.
+        allow_overwrite: Allow tags to overwrite an existing tag
     Returns:
         A Series object containing information about the commits.
     """
     if not series:
         series = Series()
+    series.allow_overwrite = allow_overwrite
     params = gitutil.LogCmd(commit_range,reverse=True, count=count,
                             git_dir=git_dir)
     stdout = command.RunPipe([params], capture=True).stdout
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 01/18] patman: Add a way of recording terminal output for testing
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 01/18] patman: Add a way of recording terminal output for testing Simon Glass
@ 2014-09-10 18:48   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:48 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 02/18] buildman: Send builder output through a function for testing
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 02/18] buildman: Send builder output through a function " Simon Glass
@ 2014-09-10 18:49   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:49 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 03/18] buildman: Enhance basic test to check summary output
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 03/18] buildman: Enhance basic test to check summary output Simon Glass
@ 2014-09-10 18:49   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:49 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 04/18] patman: RunPipe() should not pipe stdout/stderr unless asked
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 04/18] patman: RunPipe() should not pipe stdout/stderr unless asked Simon Glass
@ 2014-09-10 18:49   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:49 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 05/18] buildman: Move the command line code into its own file
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 05/18] buildman: Move the command line code into its own file Simon Glass
@ 2014-09-10 18:50   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:50 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 06/18] buildman: Move full help code into the control module
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 06/18] buildman: Move full help code into the control module Simon Glass
@ 2014-09-10 18:50   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:50 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 07/18] patman: Provide a way to intercept commands for testing
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 07/18] patman: Provide a way to intercept commands for testing Simon Glass
@ 2014-09-10 18:50   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:50 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 08/18] buildman: Add a functional test
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 08/18] buildman: Add a functional test Simon Glass
@ 2014-09-10 18:50   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:50 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 09/18] buildman: Set up bsettings outside the control module
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 09/18] buildman: Set up bsettings outside the control module Simon Glass
@ 2014-09-10 18:50   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:50 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 10/18] buildman: Avoid looking at config file or toolchains in tests
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 10/18] buildman: Avoid looking at config file or toolchains in tests Simon Glass
@ 2014-09-10 18:53   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:53 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 11/18] buildman: Allow tests to have their own boards
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 11/18] buildman: Allow tests to have their own boards Simon Glass
@ 2014-09-10 18:53   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:53 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 12/18] buildman: Correct counting of build failures on retry
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 12/18] buildman: Correct counting of build failures on retry Simon Glass
@ 2014-09-10 18:53   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:53 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 13/18] buildman: Provide an internal option to clean the outpur dir
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 13/18] buildman: Provide an internal option to clean the outpur dir Simon Glass
@ 2014-09-10 18:53   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:53 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

Fixed up to avoid removing the tree when it doesn't exist.


On 5 September 2014 19:00, Simon Glass <sjg@chromium.org> wrote:
> For testing it is useful to clean the output directory before running a
> test. This avoids a test interfering with the results of a subsequent
> test by leaving data around.
>
> Add this feature as an optional parameter to the control logic.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
>  tools/buildman/control.py | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/tools/buildman/control.py b/tools/buildman/control.py
> index fb15ae8..2f51249 100644
> --- a/tools/buildman/control.py
> +++ b/tools/buildman/control.py
> @@ -5,6 +5,7 @@
>
>  import multiprocessing
>  import os
> +import shutil
>  import sys
>
>  import board
> @@ -78,7 +79,8 @@ def ShowActions(series, why_selected, boards_selected, builder, options):
>      print ('Total boards to build for each commit: %d\n' %
>              why_selected['all'])
>
> -def DoBuildman(options, args, toolchains=None, make_func=None, boards=None):
> +def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
> +               clean_dir=False):
>      """The main control code for buildman
>
>      Args:
> @@ -93,6 +95,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None):
>          board: Boards() object to use, containing a list of available
>                  boards. If this is None it will be created and scanned.
>      """
> +    global builder
> +
>      if options.full_help:
>          pager = os.getenv('PAGER')
>          if not pager:
> @@ -209,6 +213,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None):
>      else:
>          dirname = 'current'
>      output_dir = os.path.join(options.output_dir, dirname)
> +    if clean_dir:
> +        shutil.rmtree(output_dir)
>      builder = Builder(toolchains, output_dir, options.git_dir,
>              options.threads, options.jobs, gnu_make=gnu_make, checkout=True,
>              show_unknown=options.show_unknown, step=options.step)
> @@ -230,6 +236,9 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None):
>
>          if series:
>              commits = series.commits
> +            # Number the commits for test purposes
> +            for commit in range(len(commits)):
> +                commits[commit].sequence = commit
>          else:
>              commits = None
>
> --
> 2.1.0.rc2.206.gedb03e5
>

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 14/18] patman: Start with a clean series when needed
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 14/18] patman: Start with a clean series when needed Simon Glass
@ 2014-09-10 18:54   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:54 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 15/18] buildman: Add additional functional tests
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 15/18] buildman: Add additional functional tests Simon Glass
@ 2014-09-10 18:54   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:54 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 16/18] buildman: Expand output test to cover directory prefixes
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 16/18] buildman: Expand output test to cover directory prefixes Simon Glass
@ 2014-09-10 18:54   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:54 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 17/18] buildman: Permit branch names with an embedded '/'
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 17/18] buildman: Permit branch names with an embedded '/' Simon Glass
@ 2014-09-10 18:54   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:54 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [U-Boot] [PATCH v3 18/18] buildman: Ignore conflicting tags
  2014-09-06  1:00 ` [U-Boot] [PATCH v3 18/18] buildman: Ignore conflicting tags Simon Glass
@ 2014-09-10 18:55   ` Simon Glass
  0 siblings, 0 replies; 37+ messages in thread
From: Simon Glass @ 2014-09-10 18:55 UTC (permalink / raw)
  To: u-boot

Applied to u-boot-x86/buildman.

^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2014-09-10 18:55 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-06  1:00 [U-Boot] [PATCH v3 0/18] buildman: Expand test coverage Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 01/18] patman: Add a way of recording terminal output for testing Simon Glass
2014-09-10 18:48   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 02/18] buildman: Send builder output through a function " Simon Glass
2014-09-10 18:49   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 03/18] buildman: Enhance basic test to check summary output Simon Glass
2014-09-10 18:49   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 04/18] patman: RunPipe() should not pipe stdout/stderr unless asked Simon Glass
2014-09-10 18:49   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 05/18] buildman: Move the command line code into its own file Simon Glass
2014-09-10 18:50   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 06/18] buildman: Move full help code into the control module Simon Glass
2014-09-10 18:50   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 07/18] patman: Provide a way to intercept commands for testing Simon Glass
2014-09-10 18:50   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 08/18] buildman: Add a functional test Simon Glass
2014-09-10 18:50   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 09/18] buildman: Set up bsettings outside the control module Simon Glass
2014-09-10 18:50   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 10/18] buildman: Avoid looking at config file or toolchains in tests Simon Glass
2014-09-10 18:53   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 11/18] buildman: Allow tests to have their own boards Simon Glass
2014-09-10 18:53   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 12/18] buildman: Correct counting of build failures on retry Simon Glass
2014-09-10 18:53   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 13/18] buildman: Provide an internal option to clean the outpur dir Simon Glass
2014-09-10 18:53   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 14/18] patman: Start with a clean series when needed Simon Glass
2014-09-10 18:54   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 15/18] buildman: Add additional functional tests Simon Glass
2014-09-10 18:54   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 16/18] buildman: Expand output test to cover directory prefixes Simon Glass
2014-09-10 18:54   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 17/18] buildman: Permit branch names with an embedded '/' Simon Glass
2014-09-10 18:54   ` Simon Glass
2014-09-06  1:00 ` [U-Boot] [PATCH v3 18/18] buildman: Ignore conflicting tags Simon Glass
2014-09-10 18:55   ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.