Opened 10 years ago

Closed 10 years ago

#23 closed task (wontfix)

Python ccmsg stuff review

Reported by: mgraff Owned by: shane
Priority: medium Milestone: 01. Running, non-functional authoritative-only server
Component: ~Inter-module communication(obsolete) Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity:
Sub-Project: Feature Depending on Ticket:
Estimated Difficulty: Add Hours to Ticket:
Total Hours: Internal?:

Description

Subject says it all

Subtickets

Change History (6)

comment:1 Changed 10 years ago by shane

  • Owner set to mgraff
  • Status changed from new to assigned

I'm not sure if this was ever done. Or if a later ticket (post trunk shuffle) covers this. Michael?

comment:2 Changed 10 years ago by shane

Bump?

comment:3 Changed 10 years ago by mgraff

  • Owner changed from mgraff to shane

I don't think it had been reviewed.

Some observations:

Test coverage of negative situations is poor. We probably call the methods expecting success, but don't call any tests expecting failure due to argument problems, badly formatted messages, etc. Some of this is because the doctest thing is used -- which ensures the documentation examples work -- but we need more tests as well I think. I'm quite certain parts of this code are not in fact tested in real use due to data sizes.

comment:4 Changed 10 years ago by shane

  • Owner changed from shane to mgraff

So... what is your recommendation? Is it worthwhile having someone review it without tests, or should I assign someone to add tests to this?

comment:5 Changed 10 years ago by mgraff

  • Owner changed from mgraff to shane

My recommendation is (as per the short discussion on Jabber) to switch to using JSON, use Python's built in json module, and then not have to worry about (a lot of) this. However, the session management will still need tests, which do need to be written.

This stuff has the unique advantage though that it is rather heavily used, which means it is tested as a by-product. It should probably have direct tests though, but I'd mark them down as low/medium priority rather than high.

comment:6 Changed 10 years ago by shane

  • Resolution set to wontfix
  • Status changed from assigned to closed

Okay, we're going to move to JSON, so I'm resolving this ticket.

Note: See TracTickets for help on using tickets.