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

Stardew Vallet Tests - Restructure the tests that validate Mods + ER together, improved performance #4557

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

agilbert1412
Copy link
Collaborator

@agilbert1412 agilbert1412 commented Jan 26, 2025

What is this fixing or adding?

Discord Convo between Black Sliver and myself starts here: https://discord.com/channels/731205301247803413/1214608557077700720/1333158359632969810

Short version is:

  • Reduce the scope of the test (Don't test mods that don't interact with ER, don't test ER settings that are subsets of Buildings)
  • Reduce the amount of things tested (No need to care about stray items, the other tests check that already)
  • Unroll the subtests into individual tests to play better with multithreading environments, leading to increased global performance for them.

Final improvement is about 65% on that one specific test, that took about 15 seconds out of the 90s test run. So globally, for stardew tests on my machine, it's nearly 10% improvement, probably better when multithreaded

How was this tested?

How do you think lmao

If this makes graphical changes, please attach screenshots.

Before:
image
After:
image

…mprove total performance and performance on individual tests for threading purposes
@github-actions github-actions bot added the waiting-on: peer-review Issue/PR has not been reviewed by enough people yet. label Jan 26, 2025
@agilbert1412 agilbert1412 added the is: refactor/cleanup Improvements to code/output readability or organizization. label Jan 26, 2025
Copy link
Contributor

@Jouramie Jouramie left a comment

Choose a reason for hiding this comment

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

Nitpicking on typing, the rest LGTM.

worlds/stardew_valley/test/mods/TestMods.py Outdated Show resolved Hide resolved
@agilbert1412
Copy link
Collaborator Author

A fix to the "less maintainability" issue discussed in Discord will be available in #4560 once this is merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is: refactor/cleanup Improvements to code/output readability or organizization. waiting-on: peer-review Issue/PR has not been reviewed by enough people yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants