Test framework: fix the old test bugs in newview's tests

indra/llcommon/lltracerecording.cpp

@@ -117,6 +117,17 @@ void Recording::handleReset()
 {
     LL_PROFILE_ZONE_SCOPED_CATEGORY_STATS;
 #if LL_TRACE_ENABLED
+    // A running recording that is the newest on its thread gathers in a
+    // buffer of its own, which update() leaves where it is, and every read
+    // adds in. Brought up to date first -- what it holds handed down to the
+    // recordings beneath this one, and into this one's own, which the reset
+    // then clears -- or a reset while running would keep it.
+    if (isStarted() && mActiveBuffers && LLTrace::get_thread_recorder())
+    {
+        AccumulatorBufferGroup* buffers = mBuffers.write();
+        LLTrace::get_thread_recorder()->deactivate(buffers);
+        mActiveBuffers = LLTrace::get_thread_recorder()->activate(buffers);
+    }
     mBuffers.write()->reset();
 
     mElapsedSeconds = F64Seconds(0.0);

indra/llcommon/tests/lltrace_test.cpp

@@ -88,8 +88,14 @@ namespace
         }
     };
 
+    // what drink_coffee() has raised the level to
+    static F32 sCaffeineLevel(0.f);
+
     struct trace
     {
+        // Each case drinks from nothing, whichever ran before it.
+        trace() { sCaffeineLevel = 0.f; }
+
         // before mRecorder, so it is put back after mRecorder is gone
         ThreadRecorderRestore mRestore;
         ThreadRecorder        mRecorder;
@@ -99,7 +105,6 @@ namespace
     static SampleStatHandle<F32Milligrams> sCaffeineLevelStat("caffeinelevel", "Coffee buzz quotient");
     static EventStatHandle<S32Ounces> sOuncesPerCup("cupsize", "Large, huge, or ginormous");
 
-    static F32 sCaffeineLevel(0.f);
     const F32Milligrams sCaffeinePerOz(18.f);
 
     static void drink_coffee(S32 num_cups, S32Ounces cup_size)
@@ -182,4 +187,30 @@ namespace
         }
     }
 
+    // The newest recording running on a thread gathers in a buffer of its
+    // own until something brings it up to date. Restarted while it runs, it
+    // starts from nothing all the same, and what it had gathered still
+    // reaches the older recording running beneath it.
+    TEST_CASE_METHOD(trace, "LLTrace: a running recording restarted starts from nothing and the one beneath keeps all", "[LLTrace]")
+    {
+        Recording beneath;
+        Recording newest;
+        beneath.start();
+        newest.start();
+        drink_coffee(2, S32GrandeCup(1));
+        newest.restart();
+        {
+            INFO("nothing counted once restarted");
+            CHECK(newest.getSum(sCupsOfCoffeeConsumed) == 0);
+            CHECK(newest.getSum(sOuncesPerCup) == S32Ounces(0));
+        }
+        drink_coffee(1, S32TallCup(1));
+        UNSCOPED_INFO("only what came after the restart");
+        CHECK(newest.getSum(sCupsOfCoffeeConsumed) == 1);
+        UNSCOPED_INFO("the one beneath kept what came before it too");
+        CHECK(beneath.getSum(sCupsOfCoffeeConsumed) == 3);
+        newest.stop();
+        beneath.stop();
+    }
+
 }

indra/newview/alfeedback.cpp

@@ -109,8 +109,13 @@ std::optional<ALFeedback::Endpoint> ALFeedback::endpointFromDsn(std::string_view
         return std::nullopt;
     }
 
-    const size_t at = rest.find('@');
-    if (at == std::string_view::npos)
+    // The key is the address's user part, which comes before the host, in
+    // what ends at the first '/': an '@' after it is the path's. One user
+    // part: a second would make what follows it the host the address names,
+    // whatever the host below was taken to be.
+    const std::string_view authority = rest.substr(0, rest.find('/'));
+    const size_t           at        = authority.find('@');
+    if (at == std::string_view::npos || authority.find('@', at + 1) != std::string_view::npos)
     {
         return std::nullopt;
     }

indra/newview/llviewerassetstats.cpp

@@ -273,13 +273,19 @@ void LLViewerAssetStats::getStat(LLTrace::Recording& rec, T& req, LLViewerAssetS
         || rec.getSampleCount(sDequeued[cat])
         || rec.getSampleCount(sResponse[cat]))
     {
+        // With nothing answered, or nothing fetched, there is nothing to take
+        // the least, the most or the mean of: zeros, rather than what an
+        // empty minimum, maximum and mean come out as (the largest float,
+        // the smallest, and NaN).
+        const S32 responses = rec.getSampleCount(sResponse[cat]);
+        const S32 fetches   = rec.getSampleCount(sBytesFetched[cat]);
         req .enqueued(rec.getSampleCount(sEnqueued[cat]))
             .dequeued(rec.getSampleCount(sDequeued[cat]))
-            .resp_count(rec.getSampleCount(sResponse[cat]))
-            .resp_min(rec.getMin(sResponse[cat]).value())
-            .resp_max(rec.getMax(sResponse[cat]).value())
-            .resp_mean(rec.getMean(sResponse[cat]).value())
-            .resp_mean_bytes(rec.getMean(sBytesFetched[cat]));
+            .resp_count(responses)
+            .resp_min(responses ? rec.getMin(sResponse[cat]).value() : 0.0)
+            .resp_max(responses ? rec.getMax(sResponse[cat]).value() : 0.0)
+            .resp_mean(responses ? rec.getMean(sResponse[cat]).value() : 0.0)
+            .resp_mean_bytes(fetches ? rec.getMean(sBytesFetched[cat]) : 0.0);
     }
 }
 

indra/newview/tests/alagentlocationformat_test.cpp

@@ -202,9 +202,21 @@ namespace
     TEST_CASE_METHOD(alagentlocationformat_data, "ALAgentLocationFormat: only LANDMARK truncates a long parcel name", "[ALAgentLocationFormat]")
     {
         const std::string long_name(200, 'y');
-        ALAgentLocationFormat::format(out, LLAgentUI::LOCATION_FORMAT_NORMAL,
-                                      long_name, "Region", "", 0, 0, 0);
+        const auto shown = [&](LLAgentUI::ELocationFormat format) {
+            ALAgentLocationFormat::format(out, format, long_name, "Region", "General", 0, 0, 0);
+            return out;
+        };
         UNSCOPED_INFO("NORMAL does not truncate");
-        CHECK(out == long_name + ", Region");
+        CHECK(shown(LLAgentUI::LOCATION_FORMAT_NORMAL) == long_name + ", Region");
+        UNSCOPED_INFO("NORMAL_COORDS does not truncate");
+        CHECK(shown(LLAgentUI::LOCATION_FORMAT_NORMAL_COORDS) == long_name + " (0, 0, 0)");
+        UNSCOPED_INFO("NO_MATURITY does not truncate");
+        CHECK(shown(LLAgentUI::LOCATION_FORMAT_NO_MATURITY) == long_name + ", Region (0, 0, 0)");
+        UNSCOPED_INFO("NO_COORDS does not truncate");
+        CHECK(shown(LLAgentUI::LOCATION_FORMAT_NO_COORDS) == long_name + ", Region - General");
+        UNSCOPED_INFO("FULL does not truncate");
+        CHECK(shown(LLAgentUI::LOCATION_FORMAT_FULL) == long_name + ", Region (0, 0, 0) - General");
+        UNSCOPED_INFO("LANDMARK does");
+        CHECK(shown(LLAgentUI::LOCATION_FORMAT_LANDMARK) == std::string(ALAgentLocationFormat::LANDMARK_NAME_MAX_BYTES, 'y'));
     }
 }

indra/newview/tests/alcrashreporter_test.cpp

@@ -121,15 +121,18 @@ namespace
         const LLVector3 position(128.4f, -63.6f, 22.5f);
         char buffer[64];
         size_t length = ALCrashReporter::locationTag(buffer, sizeof(buffer), "Hippotropolis", position);
+        REQUIRE(length < sizeof(buffer));
         CHECK(std::string(buffer, length) == "Hippotropolis/128/-64/23");
         CHECK(buffer[length] == '\0');
 
         char small[12];
         length = ALCrashReporter::locationTag(small, sizeof(small), "Hippotropolis", position);
+        REQUIRE(length < sizeof(small));
         CHECK(std::string(small, length) == "Hippotropol");
         CHECK(small[length] == '\0');
 
         length = ALCrashReporter::locationTag(small, sizeof(small), "Ahern", position);
+        REQUIRE(length < sizeof(small));
         UNSCOPED_INFO("cut in the numbers");
         CHECK(std::string(small, length) == "Ahern/128/-");
     }

indra/newview/tests/alcurvemodel_test.cpp

@@ -163,6 +163,7 @@ namespace
 
         mCurve.movePoint(1, 0.95f, 0.5f);
         const auto& p = mCurve.getPoints();
+        REQUIRE(p.size() == size_t(4));
         UNSCOPED_INFO("clamped below its right neighbour");
         CHECK(p[1].mX < p[2].mX);
         UNSCOPED_INFO("clamped to exactly the gap");
@@ -882,6 +883,7 @@ namespace
         const S32 n = ALToneCurveSet::LUT_SIZE;
         std::vector<U16> texels;
         set.bake(texels, n);
+        REQUIRE((S32)texels.size() == n * 4);
 
         std::vector<F32> xs;
         for (S32 i = 0; i <= 4096; ++i)

indra/newview/tests/aldaycyclelandmarks_test.cpp

@@ -129,7 +129,8 @@ namespace
     }
 
     // Landmarks are cycle positions, so they stay inside [0, 1) even when the
-    // crossing they came from sits across the seam.
+    // crossing they came from sits across the seam. Each is found first: an
+    // absent one is left at 0, which is inside the cycle whatever happened.
     TEST_CASE_METHOD(daycycle_landmarks, "ALDayCycleLandmarks: landmarks stay inside the cycle when a crossing is across the seam", "[ALDayCycleLandmarks]")
     {
         for (S32 i = 0; i < 20; ++i)
@@ -138,6 +139,10 @@ namespace
             const auto marks = ALDayCycleLandmarks::find(sine(phase));
             CAPTURE(phase);
 
+            REQUIRE(marks.has_sunrise);
+            REQUIRE(marks.has_noon);
+            REQUIRE(marks.has_sunset);
+            REQUIRE(marks.has_midnight);
             REQUIRE(marks.sunrise >= 0.f);
             REQUIRE(marks.sunrise < 1.f);
             REQUIRE(marks.noon >= 0.f);

indra/newview/tests/alfarplane_test.cpp

@@ -354,7 +354,12 @@ namespace
         REQUIRE(funcs.find("abs(rel_pos.y)") == std::string::npos);
         UNSCOPED_INFO("the rotation is in the Environment block");
         REQUIRE(shader_source("class1/deferred/environmentBlock.glsl").find("mat3  eyeToSky;") != std::string::npos);
+        // Read, and calling the haze with the position it unprojected, so the turn's absence below is not the
+        // absence of the file.
+        const std::string haze = shader_source("class3/deferred/hazeF.glsl");
         UNSCOPED_INFO("the haze pass hands over eye space");
-        REQUIRE(shader_source("class3/deferred/hazeF.glsl").find("eyeToSky") == std::string::npos);
+        REQUIRE(haze.find("calcAtmosphericVarsLinear(pos.xyz,") != std::string::npos);
+        UNSCOPED_INFO("and does not turn it itself");
+        REQUIRE(haze.find("eyeToSky") == std::string::npos);
     }
 }

indra/newview/tests/alfeedback_test.cpp

@@ -77,6 +77,16 @@ namespace
         CHECK(endpoint->publicKey == "key");
     }
 
+    // A path may hold an '@': it is the path's, after the host, and not a
+    // user part.
+    TEST_CASE("alfeedback: an @ in a self-hosted DSN's path stays in its path", "[alfeedback]")
+    {
+        const auto endpoint = ALFeedback::endpointFromDsn("https://k@sentry.example.com/team@blue/7");
+        REQUIRE(endpoint.has_value());
+        CHECK(endpoint->url == "https://sentry.example.com/team@blue/api/7/envelope/");
+        CHECK(endpoint->publicKey == "k");
+    }
+
     TEST_CASE("alfeedback: a DSN missing a part is refused", "[alfeedback]")
     {
         UNSCOPED_INFO("no scheme");
@@ -85,6 +95,8 @@ namespace
         REQUIRE_FALSE(ALFeedback::endpointFromDsn("ftp://key@host/1"));
         UNSCOPED_INFO("no key");
         REQUIRE_FALSE(ALFeedback::endpointFromDsn("https://host/1"));
+        UNSCOPED_INFO("no key, and an @ in the path, which names no user");
+        REQUIRE_FALSE(ALFeedback::endpointFromDsn("https://sentry.example.com/team@blue/7"));
         UNSCOPED_INFO("empty key");
         REQUIRE_FALSE(ALFeedback::endpointFromDsn("https://@host/1"));
         UNSCOPED_INFO("no project");
@@ -393,7 +405,17 @@ namespace
         UNSCOPED_INFO("a host that starts like loopback");
         REQUIRE_FALSE(ALFeedback::endpointFromDsn("http://k@127.0.0.1.example.com/1"));
         UNSCOPED_INFO("loopback as a user name");
+        REQUIRE_FALSE(ALFeedback::endpointFromDsn("http://127.0.0.1@evil.example.com/1"));
+        UNSCOPED_INFO("loopback in the path");
         REQUIRE_FALSE(ALFeedback::endpointFromDsn("http://k@evil.example.com/127.0.0.1/1"));
+        // A second user part, after the key's, would make what follows it the
+        // host the address names, whatever the loopback check took for one.
+        UNSCOPED_INFO("loopback as the user name of another host");
+        REQUIRE_FALSE(ALFeedback::endpointFromDsn("http://k@127.0.0.1:8765@evil.example.com/1"));
+        UNSCOPED_INFO("IPv6 loopback as the user name of another host");
+        REQUIRE_FALSE(ALFeedback::endpointFromDsn("http://k@[::1]@evil.example.com/1"));
+        UNSCOPED_INFO("a second user part, encrypted or not");
+        REQUIRE_FALSE(ALFeedback::endpointFromDsn("https://k@o42.ingest.sentry.io@evil.example.com/4501"));
     }
 
     TEST_CASE("alfeedback: a report's logs hide capability addresses and the given words", "[alfeedback]")

indra/newview/tests/algradehistory_test.cpp

@@ -71,6 +71,7 @@ namespace
 
         const auto* t = mHistory.undo();
         REQUIRE(t != nullptr);
+        REQUIRE(t->size() == (size_t)1);
         UNSCOPED_INFO("from where the drag began");
         CHECK(t->front().mBefore.asReal() == 0.0);
         UNSCOPED_INFO("to where it ended");
@@ -153,6 +154,7 @@ namespace
 
         const auto* t = mHistory.redo();
         REQUIRE(t != nullptr);
+        REQUIRE(t->size() == (size_t)1);
         CHECK(t->front().mAfter.asReal() == 1.0);
         CHECK_FALSE(mHistory.canRedo());
         CHECK(mHistory.canUndo());
@@ -172,6 +174,7 @@ namespace
         CHECK(mHistory.depth() == (size_t)1);
         const auto* t = mHistory.undo();
         REQUIRE(t != nullptr);
+        REQUIRE(t->size() == (size_t)1);
         UNSCOPED_INFO("carrying the new values");
         CHECK(t->front().mBefore.asReal() == 1.0);
     }
@@ -189,6 +192,7 @@ namespace
         // The newest is still the newest after the drop.
         const auto* t = mHistory.undo();
         REQUIRE(t != nullptr);
+        REQUIRE(t->size() == (size_t)1);
         UNSCOPED_INFO("which is the last one recorded");
         CHECK(t->front().mAfter.asReal() == (F64)(ALGradeHistory::MAX_DEPTH + 20));
     }
@@ -297,14 +301,15 @@ namespace
         UNSCOPED_INFO("the group's step carries its name");
         CHECK(mHistory.labelOf(0) == std::string("Reset Bloom"));
         UNSCOPED_INFO("a lone write's step has none");
-        REQUIRE(mHistory.labelOf(1).empty());
+        CHECK(mHistory.labelOf(1).empty());
 
         // And a group that follows an unnamed one is not named after it.
         mHistory.beginGroup();
         mHistory.record("D", LLSD(0.0), LLSD(1.0), 9.f);
         mHistory.endGroup();
+        REQUIRE(mHistory.depth() == (size_t)3);
         UNSCOPED_INFO("an unnamed group stays unnamed");
-        REQUIRE(mHistory.labelOf(2).empty());
+        CHECK(mHistory.labelOf(2).empty());
     }
 
     // Groups nest, and the one the user started is the one the step is named
@@ -398,6 +403,7 @@ namespace
         CHECK(mHistory.depth() == (size_t)1);
         const auto* t = mHistory.undo();
         REQUIRE(t != nullptr);
+        REQUIRE(t->size() == (size_t)1);
         UNSCOPED_INFO("from the value the drag began at");
         CHECK(t->front().mBefore.asReal() == 0.0);
         CHECK(t->front().mAfter.asReal() == 2.0);
@@ -410,6 +416,7 @@ namespace
         mHistory.record("A", LLSD(1.0), LLSD(0.0), 5.1f);
         UNSCOPED_INFO("the earlier step stays");
         REQUIRE(mHistory.depth() == (size_t)1);
+        REQUIRE(mHistory.at(0).size() == (size_t)1);
         CHECK(mHistory.at(0).front().mName == std::string("B"));
     }
 
@@ -445,7 +452,7 @@ namespace
         UNSCOPED_INFO("a write moves it");
         CHECK(recorded != start);
 
-        mHistory.depth();
+        REQUIRE(mHistory.depth() == (size_t)1);
         mHistory.cursor();
         mHistory.at(0);
         mHistory.labelOf(0);
@@ -476,7 +483,9 @@ namespace
         mHistory.record("A", LLSD(0.0), LLSD(1.0), 1.f);
         mHistory.record("B", LLSD(2.0), LLSD(3.0), 5.f);
 
+        REQUIRE(mHistory.depth() == (size_t)2);
         const ALGradeHistory::Transaction shown = mHistory.at(1);
+        REQUIRE(shown.size() == (size_t)1);
         const auto* undone = mHistory.undo();
         REQUIRE(undone != nullptr);
         REQUIRE(undone->size() == shown.size());
@@ -500,12 +509,14 @@ namespace
         }
         mHistory.undo();
         mHistory.undo();
+        REQUIRE(mHistory.depth() == (size_t)3);
         UNSCOPED_INFO("in force");
         CHECK(mHistory.labelOf(0) == std::string("one"));
         UNSCOPED_INFO("the next redo");
         CHECK(mHistory.labelOf(1) == std::string("two"));
         UNSCOPED_INFO("the last redo");
         CHECK(mHistory.labelOf(2) == std::string("three"));
+        REQUIRE(mHistory.at(2).size() == (size_t)1);
         UNSCOPED_INFO("a redo step's changes");
         CHECK(mHistory.at(2).front().mName == std::string("three"));
 

indra/newview/tests/alhuddepth_gl_test.cpp

@@ -164,7 +164,6 @@ namespace
         void setUp(bool reverse)
         {
             tearDown();
-            mReverse = reverse;
             setConvention(reverse);
 
             std::map<std::string, std::string> defines;
@@ -396,7 +395,6 @@ namespace
         LLMatrix4a mSceneProjection;
         LLMatrix4a mHUDProjection;
         std::vector<LLRect> mRects;
-        bool mReverse = false;
     };
 
 
@@ -601,8 +599,26 @@ namespace
             blit();
             const std::vector<F32> before = depths(mWindow);
             clearUnderHUD(&unloaded);
+            const std::vector<F32> after = depths(mWindow);
+            // Counted, and the first that differs placed, rather than every depth printed.
+            size_t differing = 0;
+            S32 first_x = -1;
+            S32 first_y = -1;
+            for (size_t i = 0; i < after.size(); ++i)
+            {
+                if (after[i] != before[i])
+                {
+                    if (differing == 0)
+                    {
+                        first_x = (S32)(i % W);
+                        first_y = (S32)(i / W);
+                    }
+                    ++differing;
+                }
+            }
+            CAPTURE(first_x, first_y);
             UNSCOPED_INFO("nothing cleared");
-            REQUIRE(depths(mWindow) == before);
+            REQUIRE(differing == size_t(0));
             drawHUD();
             restoreUnderHUD(&unloaded);
             overlay(40, 24, 8.f);