Opened 7 years ago

Closed 7 years ago

#7499 closed defect (not a bug)

Unit test bug - mocking.py:_get_descriptor_content() handlign of 'opt' keyword

Reported by: eoinof Owned by: atagar
Priority: Low Milestone:
Component: Archived/Stem Version:
Severity: Keywords: testing
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

mocking.py:_get_descriptor_content() contains several bugs
with how the opt keyword is handled.

1)
The attr arg is converted to a dictionary, so if more
than one attribute is supplied using the opt keyword it is
overwritten by the last element.

2)
Any args supplied using the keyword opt will not be matched to
the corresponding keyword in header_template. This will cause
the template be invalid.

3)
Any args supplied in attr to ovewrite values that use opt in the header_template will not be matched. This will cause the tempalte to be invalid.

The easiest fix for all of the above would be to not allow use of the opt keyword with this function in the unit tests.

Child Tickets

Change History (3)

comment:1 Changed 7 years ago by atagar

Right on all counts. However, this would only pose a problem if a test tried to include multiple 'opt' lines, and none of the server_descriptor unit tests do.

Maybe we should simply note in get_relay_server_descriptor() and get_bridge_extrainfo_descriptor() pydocs that to have multiple opt lines the caller should omit the 'opt' keyword then call replace(keyword + ' ', 'opt %s ' % keyword) on the returned result?

comment:2 Changed 7 years ago by atagar

Keywords: testing added
Priority: normalminor

comment:3 Changed 7 years ago by atagar

Resolution: not a bug
Status: newclosed

Tor's 'opt' prefix is deprecated, ignored by tor, and should be going away in newer versions. The mock function's attr are keyword/value mappings and opt isn't really a keyword. I'm fine with leaving it to callers to insert 'opt' themselves if they want to test that it's being ignored.

Resolving, feel free to reopen if you think that there's something to do here.

Note: See TracTickets for help on using tickets.