This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Replace asserts on .Success() with assertSuccess()
ClosedPublic

Authored by kastiglione on Feb 11 2022, 9:24 PM.

Details

Summary

Replace forms of assertTrue(err.Success()) with assertSuccess(err) (added in D82759).

  • assertSuccess prints out the error's message
  • assertSuccess expresses explicit higher level semantics, both to the reader and for test failure output
  • assertSuccess seems not to be well known, using it where possible will help spread knowledge
  • assertSuccess statements are more succinct

Diff Detail

Event Timeline

kastiglione created this revision.Feb 11 2022, 9:24 PM
kastiglione requested review of this revision.Feb 11 2022, 9:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 11 2022, 9:24 PM
shafik added a subscriber: shafik.Feb 11 2022, 10:18 PM

Can you please provide a motivation in the summary, thank you!

kastiglione edited the summary of this revision. (Show Details)Feb 12 2022, 8:58 AM
kastiglione edited the summary of this revision. (Show Details)Feb 12 2022, 9:14 AM
labath accepted this revision.Feb 14 2022, 4:32 AM
labath added a subscriber: labath.

Thanks for doing this.

assertSuccess seems not to be well known

That's probably because it was introduced only recently (for all the reasons that you mention).

This revision is now accepted and ready to land.Feb 14 2022, 4:32 AM

That's probably because it was introduced only recently

Thanks for adding it! I just learned about it.

kastiglione edited the summary of this revision. (Show Details)Feb 14 2022, 8:30 AM