Change in pysim[master]: filesystem: add unit tests for encoder/decoder methods

[dexter]

dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/24012 )

Change subject: filesystem: add unit tests for encoder/decoder methods
......................................................................


Patch Set 5:

(7 comments)

https://gerrit.osmocom.org/c/pysim/+/24012/4/pySim/ts_31_102.py 
File pySim/ts_31_102.py:

https://gerrit.osmocom.org/c/pysim/+/24012/4/pySim/ts_31_102.py@294 
PS4, Line 294:     _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"}]}']
> one could use a dict and then json-serialize that. […]
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)

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


https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py 
File tests/test_files.py:

https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@22 
PS4, Line 22: def test_encode_decode(testcase, ef, verbose=False):
> As I pointed out below, this function should become a method of DecTestCase.
Done


https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@49 
PS4, Line 49: 		if not has_testvec:
> This condition looks common for both branches, I would move it below.
Done


https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@57 
PS4, Line 57: 	for testvec_json in ef.__class__._encode_decode_testvector:
> Probably makes sense to wrap each vector into self.subTest too.
I do not understand what you mean. Do you mean to define a testvector class which then contains the encode_decode_testvector[] list?


https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@71 
PS4, Line 71: 		testcase.assertEqual(testvec_json, decoded_json)
> there's pros and cons. […]
In my opinion matching the strings is the best way possible. There should not be any difference between the original input and the output.


https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@82 
PS4, Line 82: DecTestCase
> Is it only decoding or encoding and decoding together? The name looks confusing.
Done


https://gerrit.osmocom.org/c/pysim/+/24012/4/tests/test_files.py@101 
PS4, Line 101: test_encode_decode
> So you have a function that accepts a reference to the test case instance, but for some reason it's  […]
Done



-- 
To view, visit https://gerrit.osmocom.org/c/pysim/+/24012
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I02d884547f4982e0b8ed7ef21b8cda75237942e2
Gerrit-Change-Number: 24012
Gerrit-PatchSet: 5
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-CC: Falkenber9 <robert.falkenberg at tu-dortmund.de>
Gerrit-Comment-Date: Thu, 27 May 2021 09:55:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge at osmocom.org>
Comment-In-Reply-To: fixeria <vyanitskiy at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210527/2707b59f/attachment.htm>