test_clobberer.py has failing tests

RESOLVED FIXED

Status

Release Engineering
General
RESOLVED FIXED
3 years ago
2 months ago

People

(Reporter: mrrrgn, Assigned: mrrrgn)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Running clobberer tests against the current server implementation has failing tests. A rewrite of the tests altogether seems pertinent.

roott@vagrant-ubuntu-trusty-64:/vagrant/clobberer-fix/tools/clobberer# python test_clobberer.py
..FF.F.......
======================================================================
FAIL: testSlaveClobberOther (__main__.TestClobber)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_clobberer.py", line 322, in testSlaveClobberOther
    self.assert_('linux_build:Server is forcing a clobber' in data, data)
AssertionError: Checking clobber URL: http://localhost:8001/index.php?master=master01&slave=slave01&builddir=mybuilder&branch=branch1&buildername=My+Builder
Server gave us {'mybuilder': (1413161854, 'testuser')}
mybuilder:Our last clobber date:  2014-10-13 00:57:35
mybuilder:Server clobber date:    2014-10-13 00:57:34


======================================================================
FAIL: testSlaveClobberRelease (__main__.TestClobber)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_clobberer.py", line 378, in testSlaveClobberRelease
    self.assert_('linux_build:Server is forcing' in data, data)
AssertionError: Checking clobber URL: http://localhost:8001/index.php?master=master01&slave=slave01&builddir=linux_build&branch=branch2&buildername=Linux+Release+Build
Server gave us {}
linux_build:Our last clobber date:  2014-10-13 00:57:26
linux_build:Server clobber date:    None


======================================================================
FAIL: testSlaveClobberReleaseOtherBranch (__main__.TestClobber)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_clobberer.py", line 396, in testSlaveClobberReleaseOtherBranch
    self.assert_('linux_build:Server is forcing' in data, data)
AssertionError: Checking clobber URL: http://localhost:8001/index.php?master=master01&slave=slave01&builddir=linux_build&branch=branch1&buildername=Linux+Release+Build
Server gave us {}
linux_build:Our last clobber date:  2014-10-13 00:57:30
linux_build:Server clobber date:    None


----------------------------------------------------------------------
Ran 13 tests in 9.698s

FAILED (failures=3)
(Assignee)

Updated

3 years ago
Assignee: nobody → winter2718
(Assignee)

Comment 1

3 years ago
Created attachment 8519308 [details] [diff] [review]
clobberer_test.diff

Explanation in commit message. These tests are simple but give a good deal of coverage. It will also allow us to turn CI back on for clobberer (it was turned off some time ago I saw due to the failing tests).
Attachment #8519308 - Flags: review?(dustin)
Comment on attachment 8519308 [details] [diff] [review]
clobberer_test.diff

Review of attachment 8519308 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, although I may not be the best person to review (I've never really dealt with the client).

::: clobberer/test_clobberer.py
@@ +13,3 @@
>  
>      def tearDown(self):
> +        os.rmdir(self.outer_dir)

Why not use `shutil.rmtree` here?  `rmdir` assumes the directory is empty..
Attachment #8519308 - Flags: review?(dustin) → review+
(Assignee)

Comment 3

3 years ago
(In reply to Dustin J. Mitchell [:dustin] from comment #2)
> Comment on attachment 8519308 [details] [diff] [review]
> clobberer_test.diff
> 
> Review of attachment 8519308 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good to me, although I may not be the best person to review (I've
> never really dealt with the client).
> 
> ::: clobberer/test_clobberer.py
> @@ +13,3 @@
> >  
> >      def tearDown(self):
> > +        os.rmdir(self.outer_dir)
> 
> Why not use `shutil.rmtree` here?  `rmdir` assumes the directory is empty..

Good point. Changing before land.
(Assignee)

Updated

3 years ago
Attachment #8519308 - Flags: checked-in+
(Assignee)

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Component: Tools → General
Product: Release Engineering → Release Engineering
You need to log in before you can comment on or make changes to this bug.