Skip to content

chore: fix failing unity tests on windows player builds - #294

Merged
KishanPRao merged 4 commits into
masterfrom
chore/fix-unity-tests-windows
Jul 7, 2026
Merged

chore: fix failing unity tests on windows player builds#294
KishanPRao merged 4 commits into
masterfrom
chore/fix-unity-tests-windows

Conversation

@KishanPRao

@KishanPRao KishanPRao commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes tests that passed in the Editor but failed with Player, fixed a bug in the Windows native client which was causing mishandling of legacy attributes, guard a few tests that should only run in the Editor or Android.

Changes

  • NativeClient: GetScopedAttributes no longer returns early when there's no scoped list, so legacy attributes are still migrated on upgrade. Save PlayerPrefs after removing legacy keys.
  • Renamed HasScopedAttributesEmpty to HasScopedAttributes, which returns true when attributes exist.
  • ScopedNativeAttributesTests: clear PlayerPrefs in [SetUp] so tests don't inherit state from earlier Player runs.
  • BacktraceClientDisableTests: skip the DisableInEditor tests in Player builds, should be Editor only.
  • BacktraceStackTraceTests: skip the Android stack trace tests in Player builds, should be Android/Editor only

@KishanPRao
KishanPRao requested a review from melekr July 2, 2026 14:24
@KishanPRao KishanPRao self-assigned this Jul 2, 2026

@melekr melekr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice cleanup. The platform specific skips look appropriate: Android stack trace parsing is compiled only for Android/Editor and the DisableInEditor assertions are editor only by design.

One small suggestion: GetScopedAttributes()now reads legacy PlayerPrefs attributes even when the scoped key list is missing, but the legacy migration path deletes those old PlayerPrefs keys without calling PlayerPrefs.Save(). Since this is part of the Windows startup/minidump flow, an explicit save after deleting legacy keys would make the cleanup deterministic and avoid relying on a later PlayerPrefs flush.

Maybe worth adding PlayerPrefs.Save() at the end of ScopedNativeAttributesTests.CleanState() after PlayerPrefs.DeleteAll() in Player test runs?

@KishanPRao

Copy link
Copy Markdown
Contributor Author

Nice cleanup. The platform specific skips look appropriate: Android stack trace parsing is compiled only for Android/Editor and the DisableInEditor assertions are editor only by design.

One small suggestion: GetScopedAttributes()now reads legacy PlayerPrefs attributes even when the scoped key list is missing, but the legacy migration path deletes those old PlayerPrefs keys without calling PlayerPrefs.Save(). Since this is part of the Windows startup/minidump flow, an explicit save after deleting legacy keys would make the cleanup deterministic and avoid relying on a later PlayerPrefs flush.

Maybe worth adding PlayerPrefs.Save() at the end of ScopedNativeAttributesTests.CleanState() after PlayerPrefs.DeleteAll() in Player test runs?

Good suggestions!
I've made changes to perform a Save on the prefs, if legacy attributes were processed & deleted from prefs.

For the Player test runs, since we run CleanState before every test, the in-memory store should always be empty. Save would write into disk, but this would not be that useful here, since we always read from a clean state, and it'd just add unused I/O operations. I'd opt to avoid adding it.

@KishanPRao KishanPRao changed the title chore: fix unity tests windows chore: fix failing unity tests on windows player builds Jul 6, 2026
@KishanPRao
KishanPRao marked this pull request as ready for review July 7, 2026 16:57
@KishanPRao
KishanPRao merged commit cb1cadb into master Jul 7, 2026
18 of 20 checks passed
@KishanPRao KishanPRao mentioned this pull request Jul 9, 2026
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