Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#3889 closed enhancement (wontfix)

Extend gtest XXX_THROW to check messages

Reported by: fdupont Owned by: UnAssigned
Priority: medium Milestone: Outstanding Tasks
Component: Unclassified Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

The gtest XXX_THROW macros check the type of the exception but not the associated message (aka the what() method.
The idea is to extend these macros so:

  • we can discriminate between different causes of a particular exception
  • we can check the message matches what is expected.

The branch will be for the lib/exceptions/tests/exceptions_unittest.cc but in a second phase at a lower priority the new macros will replace most of already existing XXX_THROWs.

Subtickets

Change History (11)

comment:1 Changed 4 years ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

comment:2 Changed 4 years ago by fdupont

Branch pushed. Note the macro is expanded by gtest when it fails but:

  • it can't be fixed without going into gtest internals
  • it should not happen as unit tests are not supposed to fail
  • the important information (got and expected messages with the file and line numbers) is given before so it is just a noise issue.

Ready for review. The new macros are {EXPECT,ASSERT}_THROW_WITH and the third/new argument is the expected message (usually a string but the code accepts anything which can go in an outstream). The new file is src/lib/util/unittests/test_exceptions.h.

comment:3 Changed 4 years ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

comment:4 Changed 4 years ago by fdupont

Changed the name of the new include to exceptions.h as it is not ambiguous (includes use relative paths to the src/lib top directory).

comment:5 Changed 4 years ago by fdupont

I finished the src/lib so if someone wants to review the few improvements I made in #3899...

Last edited 4 years ago by fdupont (previous) (diff)

comment:6 Changed 4 years ago by fdupont

  • Priority changed from high to low

Lower the priority...

comment:7 Changed 4 years ago by fdupont

  • Priority changed from low to medium

comment:8 Changed 4 years ago by tomek

  • Milestone changed from Kea-proposed to DHCP Outstanding Tasks

comment:9 follow-up: Changed 4 years ago by stephen

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

This was discussed at the team meeting of 26 August 2015. The consensus was that, although an interesting idea, these macros would be rarely - if ever - used. For this reason, it was felt that this change should not be merged into master and the ticket should be closed.

comment:10 in reply to: ↑ 9 Changed 4 years ago by fdupont

Replying to stephen:

This was discussed at the team meeting of 26 August 2015. The consensus was that, although an interesting idea, these macros would be rarely - if ever - used. For this reason, it was felt that this change should not be merged into master and the ticket should be closed.

=> even if the macros are not merged we should still check whether exception messages match expected values. I strongly suggest to add some lines in the coding guidelines saying for instance that this should be done when the same exception can come with very different messages...

comment:11 Changed 4 years ago by tomek

  • Milestone changed from DHCP Outstanding Tasks to Outstanding Tasks

Milestone renamed

Note: See TracTickets for help on using tickets.