<p><a href="https://gerrit.osmocom.org/c/pysim/+/24012">View Change</a></p><p>7 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/pysim/+/24012/4/pySim/ts_31_102.py">File pySim/ts_31_102.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/24012/4/pySim/ts_31_102.py@294">Patch Set #4, Line 294:</a> <code style="font-family:monospace,monospace">    _encode_decode_testvector = ['{"prot_scheme_id_list": [{"priority": 0, "identifier": 2, "key_index": 1}, {"priority": 1, "identifier": 1, "key_index": 2}, {"priority": 2, "identifier": 0, "key_index": 0}], "hnet_pubkey_list": [{"hnet_pubkey_identifier": 27, "hnet_pubkey": "0272da71976234ce833a6907425867b82e074d44ef907dfb4b3e21c1c2256ebcd1"}, {"hnet_pubkey_identifier": 30, "hnet_pubkey": "5a8d38864820197c3394b92613b20b91633cbd897119273bf8e4a6f4eec0a650"}]}']</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">one could use a dict and then json-serialize that. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">The json line is what would also work in the shell. I think it is important to use the same format that would also work in the shell. (I was starting with dict format as test vector but quickly noticed that the dict not always matched the json I had in mind)</p><p style="white-space: pre-wrap; word-wrap: break-word;">I agree that this line is not easy to read but you can copy+paste it into pySim-shell and it will work. I don't think there is much won when I now start to split up the strings to match any coding style cosmetics</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py">File tests/test_files.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@22">Patch Set #4, Line 22:</a> <code style="font-family:monospace,monospace">def test_encode_decode(testcase, ef, verbose=False):</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">As I pointed out below, this function should become a method of DecTestCase.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@49">Patch Set #4, Line 49:</a> <code style="font-family:monospace,monospace">            if not has_testvec:</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This condition looks common for both branches, I would move it below.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@57">Patch Set #4, Line 57:</a> <code style="font-family:monospace,monospace">       for testvec_json in ef.__class__._encode_decode_testvector:</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Probably makes sense to wrap each vector into self.subTest too.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I do not understand what you mean. Do you mean to define a testvector class which then contains the encode_decode_testvector[] list?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@71">Patch Set #4, Line 71:</a> <code style="font-family:monospace,monospace">             testcase.assertEqual(testvec_json, decoded_json)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">there's pros and cons. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">In my opinion matching the strings is the best way possible. There should not be any difference between the original input and the output.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@82">Patch Set #4, Line 82:</a> <code style="font-family:monospace,monospace">DecTestCase</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Is it only decoding or encoding and decoding together? The name looks confusing.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@101">Patch Set #4, Line 101:</a> <code style="font-family:monospace,monospace">test_encode_decode</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">So you have a function that accepts a reference to the test case instance, but for some reason it's  […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/pysim/+/24012">change 24012</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/pysim/+/24012"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: pysim </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I02d884547f4982e0b8ed7ef21b8cda75237942e2 </div>
<div style="display:none"> Gerrit-Change-Number: 24012 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Falkenber9 <robert.falkenberg@tu-dortmund.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 27 May 2021 09:55:33 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Comment-In-Reply-To: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>