Use of parse.urlencode with dict in nova/tests/unit/scheduler/client/test_report.py can result in unpredictable query strings and thus unreliable tests

Bug #1747001 reported by Chris Dent
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
High
Chris Dent

Bug Description

In nova/tests/unit/scheduler/client/test_report.py there are several tests which confirm the URLs that get passed to the placement service. These create query strings by using code like:

        expected_url = '/allocation_candidates?%s' % parse.urlencode(
            {'resources': 'MEMORY_MB:1024,VCPU:1',
             'required': 'CUSTOM_TRAIT1',
             'limit': 1000})

This results in a query string that will have an unpredictable order. Similarly, the code which is doing the actual query string creation is using the same form.

Most of the time the results are the same, and the tests pass, but sometimes they do not.

There are at least two potential ways to work around this:

* build the query strings using a sequence of tuples and set the 'doseq' param to urlencode to True. This will preserve order.
* Parse the expected_url's query params in the tests back to a dict and compare dicts

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

Fix proposed to branch: master
Review: https://review.openstack.org/540420

Changed in nova:
assignee: nobody → Chris Dent (cdent)
status: Triaged → In Progress
Matt Riedemann (mriedem)
Changed in nova:
importance: Low → High
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.openstack.org/540420
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=cab811b89030d6e49f8ef85e29783fce94c763fc
Submitter: Zuul
Branch: master

commit cab811b89030d6e49f8ef85e29783fce94c763fc
Author: Chris Dent <email address hidden>
Date: Fri Feb 2 15:34:59 2018 +0000

    Don't rely on parse.urlencode in url comparisons

    The tests for allocation_candidates query parameters in test_report.py
    relied on urlencode with a dict arg to create the expected_url that is
    compared with the actual URL that the report client creates when
    communicating with the placement service.

    This is risky because the ordering of the query parameters is
    not reliable when the source data is a dict. This change expands
    the tests where it was used to do more explicit comparisons.

    Change-Id: I2e51a4574b20c0634ad83a53c0e68261bbf0ac82
    Closes-Bug: #1747001

Changed in nova:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 17.0.0.0rc1

This issue was fixed in the openstack/nova 17.0.0.0rc1 release candidate.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.