Bug 89564 – Add master.cfg unittest to help migration - pass BuildStep instances instead of BuildStep subclasses

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Csaba Osztrogonác]

See https://bugs.webkit.org/show_bug.cgi?id=89001 for details.

[Csaba Osztrogonác]

Created attachment 148551 [details]
Patch

[Csaba Osztrogonác]

Here is an example why this unittest useful.

Let's try to fix CheckOutSource buildstep:
-        self.addStep(CheckOutSource)
+        self.addStep(CheckOutSource())

We will get the following error message:

======================================================================
FAIL: test_builder00_step01 (__main__.BuildStepsConstructorTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./mastercfg_unittest.py", line 351, in doTest
    self.fail("Error during instantiation %s buildstep for %s builder: %s\n" % (buildStepName, builder['name'].encode('ascii', 'ignore'), e))
AssertionError: Error during instantiation CheckOutSource buildstep for Apple SnowLeopard Release (Build) builder: __init__() got multiple values for keyword argument 'mode'


======================================================================
FAIL: test_builder01_step01 (__main__.BuildStepsConstructorTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./mastercfg_unittest.py", line 351, in doTest
    self.fail("Error during instantiation %s buildstep for %s builder: %s\n" % (buildStepName, builder['name'].encode('ascii', 'ignore'), e))
AssertionError: Error during instantiation CheckOutSource buildstep for Apple SnowLeopard Debug (Build) builder: __init__() got multiple values for keyword argument 'mode'

...
[snip]
...

FAILED (failures=42)


It means we handle mode incorrect in CheckoutSource's constructor. And here is the solution which pass unittests too: https://bugs.webkit.org/show_bug.cgi?id=89215

[Csaba Osztrogonác]

Created attachment 148555 [details]
Patch

Minor fix: Don't pass the whole builder object to doTest, but only the name of the builder.

[Csaba Osztrogonác]

ping review?

[Tony Chang]

Comment on attachment 148555 [details]
Patch

I don't really know the history of this unit test file, but the code seems fine.

[Tony Chang]

Comment on attachment 148555 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148555&action=review

> Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:348
> +                (buildStepFactory, kwargs) = step

Nit: no () on the left side of the assignment.

[Csaba Osztrogonác]

Committed r121328: <http://trac.webkit.org/changeset/121328>

[Csaba Osztrogonác]

Comment on attachment 148555 [details]
Patch

Landed with the nit fix.