Attention is currently required from: laforge.

View Change

6 comments:

• Patchset:

  • Patch Set #6:

    I think what we see from the several last few iterations of this patchset is that there is a lack of […]

    I also think that we should have some specific unittests. I have now created a comprehensive unittest that verifies the encoding of all three file types in all its variants. The tests also verify if total_len is correctly passed. I also verified that the tests detect bugs that were overlooked in the current patchset.

• File pySim/filesystem.py:

  • Patch Set #6, Line 746: def __get_size(self, total_len: Optional[int] = None) -> int:

    must be `-> Optional[int]` as it also may return `None`

    Done

  • Patch Set #6, Line 1054: def __get_rec_len(self, total_len: Optional[int] = None) -> int:

    likewise: `-> Optional[int]`

    Done

  • Patch Set #6, Line 1099: return b2h(t.to_tlv())

    I think we have overlooked files that use _tlv. Shouldn't padd to total_len here?

  • Patch Set #6, Line 1274:

    here we still don't return an Optional[int] but a dict? I think that's inconsistent comparing the _ […]

    Done

  • Patch Set #6, Line 1303:

    and this will again break. […]

    Done

To view, visit change 38195. To unsubscribe, or for help writing mail filters, visit settings.