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

[Test] Add test for final boss fight phase switch #3847

Merged
merged 5 commits into from
Sep 2, 2024

Conversation

MokaStitcher
Copy link
Collaborator

@MokaStitcher MokaStitcher commented Aug 27, 2024

What are the changes the user will see?

No changes hopefully, this just adds a test and makes a tiny refactor along the way

Why am I making these changes?

There was a test marked as 'todo' to check that the final boss switched phase properly when its final shield gets broken.
Since there have been issues in the past with it not working with status damage, tests were added for both status and direct damage

What are the changes from a developer perspective?

  • Implemented test marked as 'todo' for final boss switching to its second phase in src/test/final_boss.test.ts
  • This required being able to skip through the dialog after the first phase. To do this I updated the helper function gameManager.runToFinalBossEncounter to mock ui.shouldSkipDialogue instead of what it was doing before, so that all dialog gets skipped and not just the one before the fight
  • This meant in ui.showDialog the function had to actually be called (the code was already doing the same checks as shouldSkipDialogue rather than calling it), so I did that

Screenshots/Videos

N/A

How to test the changes?

npm run test src/test/final_boss.test.ts

You could play a bit and make sure the dialogue gets skipped if the setting is enabled like before

Checklist

  • I'm using beta as my base branch
  • There is no overlap with another PR?
  • The PR is self-contained and cannot be split into smaller PRs?
  • Have I provided a clear explanation of the changes?
  • Have I considered writing automated tests for the issue?
  • If I have text, did I make it translatable and add a key in the English locale file(s)?
  • Have I tested the changes (manually)?
    • Are all unit tests still passing? (npm run test)
  • Are the changes visual?
    • Have I provided screenshots/videos of the changes?

@MokaStitcher MokaStitcher requested a review from a team as a code owner August 27, 2024 10:04
@DayKev
Copy link
Collaborator

DayKev commented Aug 27, 2024

Couple comments/changes here: MokaStitcher#1

@Madmadness65 Madmadness65 added the Tests Automated tests related label Aug 29, 2024
@Tempo-anon Tempo-anon merged commit dcb03f4 into pagefaultgames:beta Sep 2, 2024
4 checks passed
@MokaStitcher MokaStitcher deleted the final-boss-test branch September 20, 2024 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tests Automated tests related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants