diff --git a/scalcs/scalcsio.py b/scalcs/scalcsio.py index 63c27f1..7a851ac 100644 --- a/scalcs/scalcsio.py +++ b/scalcs/scalcsio.py @@ -88,6 +88,26 @@ def _mec_record_to_mechanism(rec): # The mechanism drawing a .mec file carries # --------------------------------------------------------------------------- +def _state_name(field, number): + """The name in one 10-byte state-name field of a .mec file. + + The field is padded, but not always with the same byte: some writers pad + with spaces and others with NULs, and older files leave it blank, never + having stored the names. NUL counts as padding here too, and a field + blank after that gets a positional name, ``S``, so a mechanism + whose names were never written still loads with distinguishable states. + `number` is the state's number in the file - from 1, as the drawing + stored in the file numbers it - so S4 is the state drawn as 4. + + Previously a blank field either raised IndexError (space padding, nothing + for split() to return) or gave a name made of NULs (NUL padding, which + split() does not strip), the second silently: every state of the + mechanism got the same unprintable name. + """ + tokens = field.decode('utf-8').replace('\x00', ' ').split() + return tokens[0] if tokens else 'S{0:d}'.format(number) + + def _layout_from_board(board, statenames): """Positions of the states on the "chess board" stored with a mechanism. @@ -649,8 +669,7 @@ def mec_load(mecfile, start): statenames = [] for i in range(0, kstat): - statename = f.read(10).decode("utf-8") - statenames.append(statename.split()[0]) + statenames.append(_state_name(f.read(10), i + 1)) #print "State name:", statename #print ("\n") diff --git a/tests/test_mec_state_names.py b/tests/test_mec_state_names.py new file mode 100644 index 0000000..280b506 --- /dev/null +++ b/tests/test_mec_state_names.py @@ -0,0 +1,77 @@ +"""Tests for reading state names from a .mec file. + +Each name is stored in a 10-byte field. Some writers pad it with spaces and +some with NUL bytes, and older files leave it blank, never having stored the +names at all. A blank field falls back to a positional name, S1, S2, ..., +numbered as the states are in the file - which is also how the drawing stored +in the file numbers them. +""" +import os + +import pytest + +from scalcs import scalcsio + + +class TestStateNameField: + + @pytest.mark.parametrize('field, name', [ + (b'AR* ', 'AR*'), # padded with spaces + (b'AR*\x00\x00\x00\x00\x00\x00\x00', 'AR*'), # padded with NULs + (b'\x00\x00A2R\x00 \x00\x00', 'A2R'), # either, either side + (b'A2R junk ', 'A2R'), # first word only, as before + ]) + def test_padding_of_either_kind_is_removed(self, field, name): + assert scalcsio._state_name(field, 3) == name + + def test_a_field_padded_with_nuls_never_gives_a_name_containing_one(self): + """split() does not count NUL as whitespace, so the old reader named + such states with ten NUL bytes: the file loaded, but every state of + it had the same unprintable name.""" + assert '\x00' not in scalcsio._state_name(b'R' + b'\x00' * 9, 1) + + @pytest.mark.parametrize('field', [b' ' * 10, b'\x00' * 10, b' \x00' * 5]) + def test_a_blank_field_gets_the_state_s_number(self, field): + """Space-padded, the old reader raised IndexError; NUL-padded, it + returned ten NULs.""" + assert scalcsio._state_name(field, 4) == 'S4' + + +MEC_DIR = os.environ.get('SCALCS_MEC_DIR', '') + + +def _load(name): + path = os.path.join(MEC_DIR, name) + if not os.path.isfile(path): + pytest.skip('set SCALCS_MEC_DIR to a folder holding %s' % name) + _, meclist, _ = scalcsio.mec_get_list(path) + return scalcsio.mec_load(path, meclist[0][0]) + + +class TestRealFiles: + """Need .mec files from outside the repository; skipped without them.""" + + def test_a_file_that_stores_no_names(self): + """DREMIS01.MEC: five states, every name field blank.""" + mec = _load('DREMIS01.MEC') + assert sorted(mec.state_names) == ['S1', 'S2', 'S3', 'S4', 'S5'] + + def test_positional_names_follow_the_numbers_on_the_stored_drawing(self): + """DREMIS01.MEC is drawn as C5--C4--C3 + | | + O1--O2 so S5 is top left.""" + layout = _load('DREMIS01.MEC').layout + assert layout['S5'] == (0, 0) + assert layout['S1'] == (2, 2) + + def test_names_padded_with_nuls(self): + """qmechs.mec stores real names, padded with NUL bytes.""" + mec = _load('qmechs.mec') + assert set(mec.state_names) == {'AR*', 'A2R*', 'AR', 'A2R', 'R'} + assert mec.state_index('A2R*') == 1 + + def test_some_names_stored_and_some_blank(self): + """qmechsDC1.mec names its open states and leaves the shut ones + blank.""" + mec = _load('qmechsDC1.mec') + assert set(mec.state_names) == {'AR*', 'A2R*', 'S3', 'S4', 'S5'}