Opened 2 years ago

Closed 2 years ago

#6941 closed change (fixed)

[python-abp] Remove asserts from production code

Reported by: kvas Assignee: atudor
Priority: P4 Milestone:
Module: Sitescripts Keywords: goodfirstbug
Cc: Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):



Filter parser in python-abp includes the following snippet:

assert line.to_string().replace(' ', '') == content.replace(' ', '')

It ensures that parsing a filter followed by serialization to string produces the original filter. However, asserts in production code are not a reliable way of detecting errors because they are not executed if the script is run with optimizations (also it might not be the best if things crash in production). They are a performance cost however (except when run with optimizations).

What to change

  1. Remove the assert line from production code.
  2. Execute the same assertion in tests either by wrapping abp.filters.parser.parse_line or by a separate test (separate test seems preferable for simplicity and clarity but if there are good reasons to wrap, that option can be considered).

Change History (3)

comment:1 Changed 2 years ago by atudor

  • Owner set to atudor

comment:3 Changed 2 years ago by atudor

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.