Page MenuHomePhabricator

[tests] Add uacomment tests
ClosedPublic

Authored by Fabien on Jan 9 2019, 11:09.

Details

Summary

Checks for setting the value, max length and reserved characters.
The backported test has been modified to match our method, not throwing
exception but truncating and sanitizing the comment.

Backport of core PR11486

Depends on D2275

Test Plan
./test/functional/test_runner.py uacomment

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jasonbcox requested changes to this revision.Jan 9 2019, 17:26
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
test/functional/timing.json
3 ↗(On Diff #6567)

This file isn't part of the backport. Please remove.

This revision now requires changes to proceed.Jan 9 2019, 17:26
Fabien requested review of this revision.Jan 10 2019, 12:32
Fabien added inline comments.
test/functional/timing.json
3 ↗(On Diff #6567)

Even if this is not part of the original PR, I think it makes sense to have the timings file here:

  • This file does not exist in core. You can compare to a CMakeFile.txt ; even if this is not in the original PR you need to update it.
  • It doesn't really make the diff harder to review
  • It is tightly coupled to the added test. If the timing file is in another diff, this diff will have no value without the test.
  • It will avoid to forget updating the timing file :)
jasonbcox added inline comments.
test/functional/timing.json
3 ↗(On Diff #6567)

My only concern is reverting this change if the timing.json file is modified later becomes mildly annoying. Given your other points, I'm ok with it.

This revision is now accepted and ready to land.Jan 10 2019, 19:04

Rebase and update to match D2275 modified behavior

This revision was automatically updated to reflect the committed changes.