Skip to content
Snippets Groups Projects

Fix unicode/bytes handling in Tahoe config code

Merged Fix unicode/bytes handling in Tahoe config code
225.unicode-does-not-have-the-buffer-interface.2 into 228.tahoe-lafs-1.16.x-ci
Merged Jean-Paul Calderone requested to merge 225.unicode-does-not-have-the-buffer-interface.2 into 228.tahoe-lafs-1.16.x-ci

Fixes #225 (closed)

Note the target of this PR is #229 . Along with #231 this should make #229 green and then it can be merged into master to add Tahoe-LAFS 1.16.x testing to CI. For this branch alone many tests still fail against Tahoe-LAFS 1.16 but at least _zkapauthorizer.tests.test_plugin.ClientPluginTests.test_mismatched_ristretto_issuer passes now.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
src/_zkapauthorizer/tests/test_plugin.py
397 397 b"tub.port",
398 398 config_text.encode("utf-8"),
399 399 )
400 config_text = BytesIO()
400 # On Tahoe-LAFS <1.16, the config is written as bytes.
401 # On Tahoe-LAFS >=1.16, the config is written as unicode.
402 #
403 # After support for Tahoe <1.16 support is dropped we probably want to
404 # switch to an io.StringIO here.
405 config_text = StringIO()
  • Administrator
    Administrator @root
    Owner

    Created by: tomprince

    I guess we are depending on the fact that StringIO.StringIO automatically encodes unicode to bytes. That seems reasonable, but is maybe worth calling out. It is probably also worth calling out that this is a StringIO.StringIO (I was initially confused by the suggestion to switch to a io.StringIO when it is already a (different, as it turns out) StringIO here.

  • Administrator
    Administrator @root
    Owner

    Created by: tomprince

    Review: Approved

    This looks fine, though I've not checked the test results. There are a couple of minor clarifications that could be added to the comment.

  • Administrator
    Administrator @root
    Owner

    Created by: codecov[bot]

    Codecov Report

    Merging #232 (a0dc0de8) into 228.tahoe-lafs-1.16.x-ci (b7d9b00e) will increase coverage by 0.07%. The diff coverage is 100.00%.

    Impacted file tree graph

    @@                     Coverage Diff                      @@
##           228.tahoe-lafs-1.16.x-ci     #232      +/-   ##
============================================================
+ Coverage                     96.92%   96.99%   +0.07%     
============================================================
  Files                            49       49              
  Lines                          3999     3999              
  Branches                        510      510              
============================================================
+ Hits                           3876     3879       +3     
+ Misses                           80       78       -2     
+ Partials                         43       42       -1
    Impacted Files Coverage Δ
    src/_zkapauthorizer/tests/test_plugin.py 100.00% <100.00%> (ø)
    src/_zkapauthorizer/tests/test_client_resource.py 98.32% <0.00%> (+0.27%) :arrow_up:
    src/_zkapauthorizer/tests/fixtures.py 100.00% <0.00%> (+5.00%) :arrow_up:

    Continue to review full report at Codecov.

    Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b7d9b00...a0dc0de. Read the comment docs.

    • Please register or sign in to reply