Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix save parser test, and fix error in parsing old saves #124

Merged
merged 1 commit into from
Dec 23, 2023

Conversation

chennin
Copy link
Contributor

@chennin chennin commented Dec 1, 2023

  1. Old saves that don't have council_positions were failing to parse. Add an existence check for the data structure in _update_council_positions().

  2. The test for a full save parse did not propagate an exception back to the tester, causing false negatives (it only successfully tested the existence of the rust save parser). By setting config.CONFIG.debug_mode to True we hit code that propagates the exception, and the test properly fails when a save fails to parse.

Repro: python -m pytest test/parser_test.py -k "test_real_save" -s

  1. On v6, prints a stack trace but does NOT fail the test
  2. On this branch, does not print a stack strace and does not fail the test.
  3. On this branch, if you remove the check-and-early-return for missing council_positions, the parse prints a trace and the test fails.

The stack trace:

2023-11-30 20:34:08,493 - stellarisdashboard.parsing.timeline - INFO - nexitronawareness_1329922464 2565.06.02   - Processing council
2023-11-30 20:34:08,517 - stellarisdashboard.parsing.timeline - ERROR - nexitronawareness_1329922464 2565.06.02 Rolling back changes to database...
Traceback (most recent call last):
  File "/home/alucard/stellaris-dashboard/stellarisdashboard/parsing/timeline.py", line 60, in process_gamestate
    self._process_gamestate(db_game)
  File "/home/alucard/stellaris-dashboard/stellarisdashboard/parsing/timeline.py", line 110, in _process_gamestate
    data_processor.extract_data_from_gamestate(
  File "/home/alucard/stellaris-dashboard/stellarisdashboard/parsing/timeline.py", line 2047, in extract_data_from_gamestate
    self._update_council_positions(countries_by_id, leaders_by_id)
  File "/home/alucard/stellaris-dashboard/stellarisdashboard/parsing/timeline.py", line 2052, in _update_council_positions
    self._gamestate_dict["council_positions"]["council_positions"].items()
KeyError: 'council_positions'

This was hidden from all automated tests before!

NB I haven't actually checked the effect in the GUI of skipping _update_council_positions() - I haven't played Stellaris since before the council existed :). I'd appreciate a large, end-game 3.10.x (preferably 3.10.2!) save to add to the test cases.

Old saves that don't have council_positions were failing to parse.  Add an existence check for the data structure in _update_council_positions().

The test for a full save parse did not propagate an exception back to the tester, causing false negatives (it only successfully tested the existence of the rust save parser). By setting config.CONFIG.debug_mode to True we hit code that propagates the exception, and the test properly fails when a save fails to parse.
Copy link
Owner

@eliasdoehne eliasdoehne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@eliasdoehne eliasdoehne merged commit 4675573 into eliasdoehne:master Dec 23, 2023
3 checks passed
chennin added a commit to chennin/stellaris-dashboard that referenced this pull request Dec 23, 2023
Old saves that don't have council_positions were failing to parse.  Add an existence check for the data structure in _update_council_positions().

The test for a full save parse did not propagate an exception back to the tester, causing false negatives (it only successfully tested the existence of the rust save parser). By setting config.CONFIG.debug_mode to True we hit code that propagates the exception, and the test properly fails when a save fails to parse.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants