Make EdgePicker_DoesNotPickEdge_BothPointsBehindCamera ignore expected asserts

[thomas-tu]

Purpose of this PR

[Brief summary of the changes in this PR.]

Links

Jira:

Comments to Reviewers

[List known issues, planned work, provide any extra context for your code.]

[u-pr-agent]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪ The PR contains a single-line change in a test file, which is straightforward to understand and review.

🏅 Score: 65 The change weakens a test by ignoring all potential errors instead of specifically expecting known ones, which could hide future regressions.

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Weakened Test Setting LogAssert.ignoreFailingMessages = true is overly broad and can mask legitimate, unexpected errors in the future. This makes the test less reliable. It's preferable to revert to using the more specific LogAssert.Expect to target only the known, expected error messages. If the error messages are unstable, consider using a regular expression with LogAssert.Expect. UnityEngine.TestTools.LogAssert.ignoreFailingMessages = true;

Update review

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

[varinotmUnity]

lgtm. Should we also UnityEngine.TestTools.LogAssert.ignoreFailingMessages = false; in the cleanup of this file, and the facePicker one?

[u-pr-agent]

Suggestion: The LogAssert.ignoreFailingMessages flag is a static property that persists across tests. Failing to reset it can cause subsequent tests to unintentionally ignore critical errors. To prevent this side effect, you should restore its original value using a try...finally block, ensuring it's always reset even if the test action throws an exception. [possible issue, importance: 9]

New proposed code:

+var previousIgnoreFailingMessages = UnityEngine.TestTools.LogAssert.ignoreFailingMessages;
 UnityEngine.TestTools.LogAssert.ignoreFailingMessages = true;
-EditorSceneViewPicker.DoMouseClick(
-    CreateMouseEvent(mousePos, EventType.MouseDown, EventModifiers.None),
-    SelectMode.Edge,
-    m_PickerPreferences);
+try
+{
+    EditorSceneViewPicker.DoMouseClick(
+        CreateMouseEvent(mousePos, EventType.MouseDown, EventModifiers.None),
+        SelectMode.Edge,
+        m_PickerPreferences);
+}
+finally
+{
+    UnityEngine.TestTools.LogAssert.ignoreFailingMessages = previousIgnoreFailingMessages;
+}

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

[u-pr-agent]

Suggestion: Using LogAssert.ignoreFailingMessages = true is risky as it can hide new, unexpected errors, reducing the test's effectiveness. A more robust approach is to explicitly declare the expected log messages using LogAssert.Expect. The original approach was likely failing because the LogType was not specified; assertion failures, for example, should be expected with LogType.Assert. [general, importance: 10]

Suggested change

	UnityEngine.TestTools.LogAssert.ignoreFailingMessages = true;
	// Please adjust the LogType to match the actual log message type.
	UnityEngine.TestTools.LogAssert.Expect(LogType.Warning, "Handles.GetClosestPickingID called outside an editor OnGUI");
	UnityEngine.TestTools.LogAssert.Expect(LogType.Assert, "Assertion failed on expression: 'device.IsInsideFrame()'");

---

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

[codecov-git.colasdn.top]

Codecov Report

All modified and coverable lines are covered by tests ✅

@@           Coverage Diff           @@
##           master     #637   +/-   ##
=======================================
  Coverage   35.55%   35.55%           
=======================================
  Files         277      277           
  Lines       34889    34889           
=======================================
  Hits        12406    12406           
  Misses      22483    22483           

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.