Skip to content

Add post-run cleanup for integration tests - #168

Open
olive-tree-branch wants to merge 1 commit into
thoth-tech:mainfrom
olive-tree-branch:Bugfix/Post-run-Test-Cleanup
Open

Add post-run cleanup for integration tests#168
olive-tree-branch wants to merge 1 commit into
thoth-tech:mainfrom
olive-tree-branch:Bugfix/Post-run-Test-Cleanup

Conversation

@olive-tree-branch

@olive-tree-branch olive-tree-branch commented Jul 28, 2026

Copy link
Copy Markdown

Description

Fixed error at the end of the camera test where the x and y position would not be reset if moved. This was causing the problem of other tests not having the camera centered.

I have found another problem in the bundles test that is causing two bundles to incorrectly not be freed. There is no fix yet for that but I will update the pull request with any fixes. There is another PR that fixes this problem #153

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The changes are being tested with sktest to make sure the correct results and

Testing Checklist

  • Tested with sktest

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have requested a review for the Pull Request

Fixed error at the end of the camera test where the x and y position
would not be reset if moved. This was causing the problem of other tests
not having the camera centered

@rory-cd rory-cd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

General Information

Type of Change:

  • Bug fix

Nice and simple solution. I've tested in WSL via sktest and it completely resolves the issue. Good pickup on the bundle problem. I believe this has been addressed in another PR (two actually, I think it was a duplicate ticket).

Code Quality

  • Repository: Thoth-Tech NOT SplashKit
  • Readability: Is the code easy to read and follow? If not are there comments to help understand the code?
  • Maintainability: Can this code be easily maintained or extended in the future?

Functionality

  • Correctness: Does the code meet the requirements of the task?
  • Impact on Existing Functionality: Has the impact on existing functionality been considered and tested?

Testing

  • Test Coverage: Are unit tests provided for new or modified code?
  • Test Results: Have all tests passed?

Documentation

  • Documentation: Are both inline and applicable external documentation updated and clear?

Pull Request Details

  • PR Description: Is the problem being solved clearly described?
  • Checklist Completion: Have all relevant checklist items been reviewed and completed?

@olive-tree-branch

olive-tree-branch commented Jul 30, 2026

Copy link
Copy Markdown
Author

Was there a fix for the bundles unit test in another PR or is it worth still looking for a fix? Just wanted to check in case there's double up Added the PR link for the freeing bundles fix into the this PR

@olive-tree-branch

olive-tree-branch commented Jul 30, 2026

Copy link
Copy Markdown
Author

Will add cards to the planner for the following so this PR doesn't become too convoluted:

  • Fix brawler font link as the page does not exist anymore - already in a card
  • Find new music file to download in audio test - already in a card
  • Fix double free in texts test when running the test back to back - related to existing card above

@olive-tree-branch
olive-tree-branch force-pushed the Bugfix/Post-run-Test-Cleanup branch 2 times, most recently from a38d03a to 652bdff Compare July 31, 2026 04:07

@Rhinoatron Rhinoatron left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work on this fix. I’ve completed the second peer review and confirmed that resetting both camera coordinates before closing the window prevents the camera state from affecting later tests. The change is clear, appropriately scoped, and the first peer reviewer has also confirmed it works successfully with sktest. The other test issues have been kept separate so this PR remains focused. Approved.

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.

3 participants