Ensure that node ids are printable#4418
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4418 +/- ##
==========================================
+ Coverage 95.88% 95.89% +<.01%
==========================================
Files 111 111
Lines 25008 25039 +31
Branches 2438 2441 +3
==========================================
+ Hits 23979 24010 +31
- Misses 725 727 +2
+ Partials 304 302 -2
Continue to review full report at Codecov.
|
| else: | ||
| escaped.decode("ascii") | ||
| assert isinstance(escaped, six.text_type) | ||
| escaped.encode("ascii") |
There was a problem hiding this comment.
even with the old code its not clear what good this test does
There was a problem hiding this comment.
I think it's a cheeky way to say "this is encodabe / decodable with ascii"
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
this looks great, we need to follow up with taking off some hacks (like the null byte converted to "(null)" in the env var
|
@RonnyPfannschmidt @blueyed -- is this ok to go into |
|
@asottile |
|
features please |
This was documented before, but never enforced. Passing non-strings could have strange side-effects and enforcing a string simplifies other implementation.
|
done, rebased against features |
Resolves #4397
targeting
features:param(id=...)is a stringid=...on construction when manually passed to enforce printability