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.

Sign up for GitHub

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

Jump to bottom

Stardew Valley: Remove some duplicate code in tests #2006

Merged
merged 1 commit into from
Jul 30, 2023
Merged

Stardew Valley: Remove some duplicate code in tests #2006

merged 1 commit into from
Jul 30, 2023

Conversation

alwaysintreble
Copy link
Collaborator

What is this fixing or adding?

Removes duplicate code from Stardew Valley's test setup and optimized imports. Would be good practice to also rename the setup function as Python doesn't like functions with the same name but different signatures.

How was this tested?

Unit Tests

@agilbert1412

@ThePhar ThePhar added the is: refactor/cleanup Improvements to code/output readability or organizization. label Jul 22, 2023
@ThePhar
Copy link
Member

ThePhar commented Jul 22, 2023

image
🤔

@black-sliver
Copy link
Member

Hm, i think it'd be nicer to not rename stardew_valley.test.setup_solo_multiworld, but instead do from test.general import setup_solo_multiworld as setup_base_solo_multiworld and call it as setup_base_solo_multiworld.

agilbert1412
agilbert1412 approved these changes Jul 25, 2023
View reviewed changes
Copy link
Collaborator

@agilbert1412 agilbert1412 left a comment

Choose a reason for hiding this comment

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

Someone who knows what is what should still review this, I'm not familiar with the base test code

@Berserker66 Berserker66 merged commit cc39eec into ArchipelagoMW:main Jul 30, 2023
@alwaysintreble alwaysintreble deleted the stardew_tests branch July 30, 2023 18:42
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
@alwaysintreble @FlySniper
Stardew Valley: Import base multiworld setup in tests and use it (Arc…
f645ea4 
…hipelagoMW#2006)
kl3cks7r pushed a commit to kl3cks7r/Archipelago that referenced this pull request Dec 15, 2023
@alwaysintreble
Stardew Valley: Import base multiworld setup in tests and use it (Arc…
8f33fa0 
…hipelagoMW#2006)
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
@alwaysintreble @Jouramie
Stardew Valley: Import base multiworld setup in tests and use it (Arc…
2c8e2e7 
…hipelagoMW#2006)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agilbert1412 agilbert1412 agilbert1412 approved these changes

@Berserker66 Berserker66 Berserker66 approved these changes

Assignees
No one assigned
Labels
is: refactor/cleanup Improvements to code/output readability or organizization.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@alwaysintreble @ThePhar @black-sliver @Berserker66 @agilbert1412