Skip to content

Fix double-click handling in UI lists and improve test suite for missing MPQ assets#8273

Closed
skypher wants to merge 3 commits intodiasurgical:masterfrom
skypher:fix-menu-click-bug
Closed

Fix double-click handling in UI lists and improve test suite for missing MPQ assets#8273
skypher wants to merge 3 commits intodiasurgical:masterfrom
skypher:fix-menu-click-bug

Conversation

@skypher
Copy link
Copy Markdown
Contributor

@skypher skypher commented Nov 18, 2025

Summary

This PR contains two sets of improvements:

1. Fix double-click handling in UI lists

  • Fixed an issue with double-click handling in the UI list components

2. Gracefully handle missing MPQ assets in test suite

Modified the test suite to handle missing MPQ assets (spawn.mpq/DIABDAT.MPQ) gracefully instead of failing hard.

Changes:

  • Tests now run even when MPQ assets are not present
  • Tests requiring MPQ skip gracefully using GTEST_SKIP()
  • Added custom GoogleTest listener to track and display skip reasons
  • Clear summary shown at end explaining why tests were skipped

Benefits:

  • Test suite can run in CI/CD environments without MPQ assets
  • Clear feedback about which tests couldn't run and why
  • No more hard failures when assets are missing

Example output when MPQ is missing:

========================================
Test Skip Summary
========================================
Total tests skipped: 19

  • 19 tests skipped: MPQ assets (spawn.mpq or DIABDAT.MPQ) not found
    (18 tests automatically skipped due to test suite setup failure)
========================================

Test plan

  • All tests pass with MPQ assets present (290/290 tests)
  • All tests skip gracefully without MPQ assets with clear messages
  • Skip summary shows reasons for skipped tests

Modified test suite to handle missing MPQ assets (spawn.mpq/DIABDAT.MPQ)
gracefully instead of failing. Tests now:

- Run even when MPQ assets are not present
- Skip tests requiring MPQ assets with GTEST_SKIP()
- Display a clear summary at the end explaining why tests were skipped
- Show total count and reasons for skipped tests

Changes:
- test/main.cpp: Added custom GoogleTest listener to track and report skip reasons
- test/{inv,pack,player,timedemo,vendor,writehero}_test.cpp:
  Replaced ASSERT_TRUE(HaveMainData()) with GTEST_SKIP() for graceful skipping

This allows the test suite to run in CI/CD environments without MPQ assets
while still providing clear feedback about which tests couldn't run and why.
@skypher
Copy link
Copy Markdown
Contributor Author

skypher commented Nov 18, 2025

Wrong target repository - recreating PR on skypher/DevilutionX

@skypher skypher closed this Nov 18, 2025
@AJenbo
Copy link
Copy Markdown
Member

AJenbo commented Nov 18, 2025

if this is just to keep your fork in sync there should be a button to do so on github, or you can simply reset your local master to the upstream master and push to your fork. Doing it by merging PRs on github will create additional commits

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