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

Add tests for transitions #254

Merged
merged 5 commits into from
Aug 4, 2023

Conversation

silviot
Copy link
Contributor

@silviot silviot commented Aug 4, 2023

These tests rely on the existing object definitions.

The following transition, defined in the test, should probably be added to the existing ones.

('last', 2, 3): {
                'actor_end': 5,
                'actor_start': 2,
                'last_use': False,
                'modify_uses': [0, 0],
                'target_end': 3,
                'target_start': 3,
                'visible': 'always'
            },

Copy link
Contributor

@alecpm alecpm left a comment

Choose a reason for hiding this comment

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

I think we still need to test when to use the regular vs last transition here, as well as the modify_uses stuff, but this is a good start.

self.messages[:] = []

@pytest.fixture
def item(self, exp):
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you're using this fixture twice, maybe we move it out to the module level (or into conftest.py)?

def publish(error_msg):
self.messages.append(error_msg)
exp.publish = publish
exp.transition_config = {
Copy link
Contributor

Choose a reason for hiding this comment

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

There will be an issue with this once @jessesnyder merges PR #252 which updates the game_config.yml to use string ids to make these transition definitions a little less opaque. If we're going to depend on the existing game_config.yml maybe this could just test the existing Gooseberry Bush -> Empty Gooseberry Bush transition last_use transition?

@alecpm alecpm merged commit 701aa5f into one-hour-one-life Aug 4, 2023
2 checks passed
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