Skip to content

Revert "CHANGE: XRI test ignored as it is causing issues in the CI. I… #2399

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.

Sign up for GitHub

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

Open
josepmariapujol-unity wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Revert "CHANGE: XRI test ignored as it is causing issues in the CI. I… #2399

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Assets/Tests/InputSystem.Editor/XRIPackageTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ public IEnumerator TearDown()

[UnityTest]
[Category("Integration")]
[Ignore("ISX-2531 - This test is currently ignored as it is causing issues in the CI. It should be re-enabled once the underlying issue is resolved.")]
public IEnumerator AdddingLatestXRIPackageThrowsNoErrors()
Copy link
Copy Markdown
Contributor

@u-pr u-pr bot Mar 26, 2026

Choose a reason for hiding this comment

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

low
There is a minor typo in the method name: "Addding" should be "Adding".

Suggested change
public IEnumerator AdddingLatestXRIPackageThrowsNoErrors()
public IEnumerator AddingLatestXRIPackageThrowsNoErrors()

🤖 Helpful? 👍/👎

{
Application.logMessageReceived += HandleLog;
Copy link
Copy Markdown
Contributor

@u-pr u-pr bot Mar 26, 2026

Choose a reason for hiding this comment

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

medium
This test re-enables a check that was previously ignored due to CI issues, but it appears to still leak the HandleLog event handler. Since Application.logMessageReceived is a static event, adding a handler on line 43 without a corresponding unsubscription in the TearDown method (or a try-finally block) will cause the handler to persist across subsequent tests in the same run. This can lead to flaky failures or side effects in the rest of the test suite, which is likely the "issue in the CI" mentioned in the original ignore attribute.

Have you considered unsubscribing from this event in the TearDown method to ensure the environment is clean for other tests?

🤖 Helpful? 👍/👎

Expand Down
Loading