Unit tests shouldn't take config

[Tim Burke]

From a fresh checkout on a pretty bare VM, running unit tests leads with

   Unable to read test config /etc/swift/test.conf - file not found

Why do we expect swift to already be configured?? Let's dig around a bit...

OK, looks like that message came out of get_config [1], and *not* having that file will just get us a default config and still run everything... but that's not at all obvious from the message -- as soon as I see some out-of-band message coming out of my tests, I start to question the result, even if it "passes". And we still haven't explained *why unit tests need config at all*.

Looking for get_config under test/unit/, the only reference [2] gets fake_syslog to decide whether or not to replace logging.handlers.SysLogHandler... which more or less seems like a reasonable thing to do? The change that added it [3] referenced a couple bugs [4][5] that make a decent case for the new behavior, anyway.

So maybe we should *always* replace SysLogHandler, and make our unit tests more encapsulated. Oops, now tests don't pass:

======================================================================
FAIL: test_get_logger_sysloghandler_plumbing (test.unit.common.test_utils.TestUtils)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/vagrant/swift/test/unit/common/test_utils.py", line 1494, in test_get_logger_sysloghandler_plumbing
    syslog_handler_args)
AssertionError: Lists differ: [((), {'facility': 19, 'addres... != [((), {'facility': 19, 'addres...

First list contains 1 additional elements.
First extra element 1:
((), {'facility': 19})

- [((), {'address': '/foo/bar', 'facility': 19}), ((), {'facility': 19})]

+ [((), {'address': '/foo/bar', 'facility': 19})]

But running common/test_utils.py:TestUtils.test_get_logger_sysloghandler_plumbing in isolation passes :-( Minimal combination I've found that starts to fail is something like

  ./.unittests common/test_daemon.py:TestRunDaemon.test_run_daemon \
    common/test_utils.py:TestUtils.test_get_logger_sysloghandler_plumbing

...all of which seems to reinforce the idea that unit tests *shouldn't take config* -- we've clearly not been testing with the other config!

[1] https://github.com/openstack/swift/blob/2.14.0/test/__init__.py#L49-L73
[2] https://github.com/openstack/swift/blob/2.14.0/test/unit/__init__.py#L682-L695
[3] https://github.com/openstack/swift/commit/f7fdb9c
[4] https://bugs.launchpad.net/swift/+bug/701248
[5] https://bugs.launchpad.net/swift/+bug/819303

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/506410

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/506410
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=cc17c99e73e9ddb1768f2979074c3ec043e0a3b4
Submitter: Jenkins
Branch: master

commit cc17c99e73e9ddb1768f2979074c3ec043e0a3b4
Author: Tim Burke <email address hidden>
Date: Thu Sep 21 22:25:57 2017 +0000

    Stop reloading swift.common.utils in test_daemon

    This was causing some headaches over on feature/deep where a __eq__
    wasn't working as expected because neither self nor other was an
    instance of the class we thought we were using. Apparently, this
    also fixes some issues when using fake_syslog = True?

    There are two other places that we use reload_module, in
    test_db_replicator and test_manager, but the monkey patching isn't
    nearly as straight-forward.

    Change-Id: I94d6578e275219e9687fee2f0c7cc4f99454b77f
    Related-Bug: 1704192

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: feature/deep
Review: https://review.openstack.org/508700

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/508700
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=0c75ddf6fe5a4843fe60836b402f27cb1b83d8c5
Submitter: Zuul
Branch: feature/deep

commit 93fc9d2de86f37f62b1d6768600d0551e1b72fb6
Author: Alistair Coles <email address hidden>
Date: Wed Sep 27 16:35:27 2017 +0100

    Add cautionary note re delay_reaping in account-server.conf-sample

    Change-Id: I2c3eea783321338316eecf467d30ba0b3217256c
    Related-Bug: #1514528

commit c6aea4b3730c937c41815831a7b4d60ff2899fcb
Author: Tim Burke <email address hidden>
Date: Wed Sep 27 19:19:53 2017 +0000

    Fix intermittent failure in test_x_delete_after

    X-Delete-After: 1 is known to be flakey; use 2 instead.

    When the proxy receives an X-Delete-After header, it automatically
    converts it to an X-Delete-At header based on the current time. So far,
    so good. But in normalize_delete_at_timestamp we convert our

        time.time() + int(req.headers['X-Delete-After'])

    to a string representation of an integer and in the process always round
    *down*. As a result, we lose up to a second worth of object validity,
    meaning the object server can (rarely) respond 400, complaining that the
    X-Delete-At is in the past.

    Change-Id: Ib5e5a48f5cbed0eade8ba3bca96b26c82a9f9d84
    Related-Change: I643be9af8f054f33897dd74071027a739eaa2c5c
    Related-Change: I10d3b9fcbefff3c415a92fa284a1ea1eda458581
    Related-Change: Ifdb1920e5266aaa278baa0759fc0bfaa1aff2d0d
    Related-Bug: #1597520
    Closes-Bug: #1699114

commit 5c76b9e691166acc1f7b8483aaa3980ebc70bd3a
Author: Alistair Coles <email address hidden>
Date: Wed Sep 27 14:11:14 2017 +0100

    Add concurrent_gets to proxy.conf man page

    Change-Id: Iab1beff4899d096936c0e5915f3ec32364b3e517
    Closes-Bug: #1559347

commit b4f08b6090057897ac647ba6331a4ec867b8e3b8
Author: Jens Harbott <email address hidden>
Date: Wed Sep 27 09:10:54 2017 +0000

    Fix functest for IPv6 endpoints

    Currently the functional tests fail if the storage_url contains a quoted
    IPv6 address because we try to split on ':'.

    But actually we don't need to split hostname and port only in order to
    combine it back together lateron. Use the standard urlparse() function
    instead and work with the 'netloc' part of the URL which keeps hostname
    and port together.

    Change-Id: I64589e5f2d6fb3cebc6768dc9e4de6264c09cbeb
    Partial-Bug: 1656329

commit 53ab6f2907eff2bb90528010d881f2f87ee02505
Author: Alistair Coles <email address hidden>
Date: Tue Sep 26 11:43:53 2017 +0100

    Assert memcached connection error is logged

    Follow up to [1] - change logger mocking so that we can
    assert the memcached connection error is logged.

    [1] Related-Change: I97c5420b4b4ecc127e9e94e9d0f91fbe92a5f623

    Change-Id: I87cf4245082c5e0f0705c2c14ddfc0b5d5d89c06

commit e501ac7d2be5c11b2ed0005885c84023054ec041
Author: Matthew Oliver <email address hidden>
Date: Thu Sep 3 12:19:05 2015 +1000

    Fix memcached exception out of range stacktrace

    When a memecached server goes offline in the middle of a
    MemcahceRing...

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: feature/s3api
Review: https://review.openstack.org/512277

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: feature/s3api
Review: https://review.openstack.org/512283

[OpenStack Infra (hudson-openstack)]

Change abandoned by Alistair Coles (<email address hidden>) on branch: feature/s3api
Review: https://review.openstack.org/512283
Reason: I was just trying to get sensible topic

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/512277
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=f94d6567a7e2e8b3ca1168b4a41c42c1ee371af5
Submitter: Zuul
Branch: feature/s3api

commit 24188beb81d39790034fa0902246163a7bf54c91
Author: Samuel Merritt <email address hidden>
Date: Thu Oct 12 16:13:25 2017 -0700

    Remove some leftover threadpool cruft.

    Change-Id: I43a1a428bd96a2e18aac334c03743a9f94f7d3e1

commit 1d67485c0b935719e0c8999eb353dfd84713add6
Author: Samuel Merritt <email address hidden>
Date: Fri Apr 15 12:43:44 2016 -0700

    Move all monkey patching to one function

    Change-Id: I2db2e53c50bcfa17f08a136581cfd7ac4958ada2

commit 407f5394f0f5cb422c06b4e5b2f9fbfdb07782d1
Author: OpenStack Proposal Bot <email address hidden>
Date: Thu Oct 12 08:12:38 2017 +0000

    Imported Translations from Zanata

    For more information about this automatic import see:
    https://docs.openstack.org/i18n/latest/reviewing-translation-import.html

    Change-Id: I628cb09aa78d8e339b4762a3c9ed8aed43941261

commit 45ca39fc68cdb42b382c1638a92cc8d3cec5529a
Author: Clay Gerrard <email address hidden>
Date: Tue Oct 10 11:47:50 2017 -0700

    add mangle_client_paths to example config

    Change-Id: Ic1126fc95e8152025fccf25356c253facce3e3ec

commit 94bac4ab2fe65104d602378e8e49c37b8187a75d
Author: Tim Burke <email address hidden>
Date: Fri May 12 10:55:21 2017 -0400

    domain_remap: stop mangling client-provided paths

    The root_path option for domain_remap seems to serve two purposes:
     - provide the first component (version) for the backend request
     - be an optional leading component for the client request, which
       should be stripped off

    As a result, we have mappings like:

       c.a.example.com/v1/o -> /v1/AUTH_a/c/o

    instead of

       c.a.example.com/v1/o -> /v1/AUTH_a/c/v1/o

    which is rather bizarre. Why on earth did we *ever* start doing this?

    Now, this second behavior is managed by a config option
    (mangle_client_paths) with the default being to disable it.

    Upgrade Consideration
    =====================

    If for some reason you *do* want to drop some parts of the
    client-supplied path, add

       mangle_client_paths = True

    to the [filter:domain_remap] section of your proxy-server.conf. Do this
    before upgrading to avoid any loss of availability.

    UpgradeImpact
    Change-Id: I87944bfbf8b767e1fc36dbc7910305fa1f11eeed

commit a4a5494fd2fe8a43a5d50a21a1951266cc7c4212
Author: Alistair Coles <email address hidden>
Date: Mon Oct 9 11:33:28 2017 +0100

    test account autocreate listing format

    Related-Change: Id3ce37aa0402e2d8dd5784ce329d7cb4fbaf700d
    Change-Id: I50c22225bbebff71600bea9158bda1edd18b48b0

commit 8b7f15223cde4c19fd9cbbd97e8ad79a1b4afa8d
Author: Alistair Coles <email address hidden>
Date: Mon Oct 9 10:06:19 2017 +0100

    Add example to container-sync-realms.conf.5 man page

    Related-Change: I0760ce149e6d74f2b3f1badebac3e36da1ab7e77

    Change-Id: I129de42f91d7924c7bcb9952f17fe8a1a10ae219

commit 816331155c624c444ed123bcab412...