[Chore] (MG_Test): assert exact GL error counts - name-lifecycle regression tests per object family, plus fixtures that drain on setup and fail the test that leaks an unconsumed error

MobileGL/MG_Test/Buffer/BufferTest.cpp

@@ -8,6 +8,8 @@
 
 #include <gtest/gtest.h>
 
+#include <limits>
+
 #include "Includes.h"
 #include "Init.h"
 #include <Config.h>
@@ -20,9 +22,31 @@ using namespace MobileGL;
 
 class BufferTest : public ::testing::Test {
 protected:
-    void SetUp() override { MobileGL::Initialize(); }
+    // GL error flags are sticky per error code and the context outlives an individual test in this
+    // binary, so drain whatever an earlier test left pending - otherwise an error-code assertion
+    // here reads someone else's error. Bounded: one flag per code, so this cannot hang the suite.
+    static void DrainPendingGlErrors() {
+        for (Int drained = 0; drained < 16 && MG_Impl::GLImpl::GetError() != GL_NO_ERROR; ++drained) {
+        }
+    }
 
-    void TearDown() override {}
+    // The call under test must raise exactly the expected error and nothing more: a second pending
+    // error means one entry point queued several, which GetError() would hand out at an unrelated
+    // call site later on.
+    static void ExpectSingleGlError(GLenum expected) {
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), expected);
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "the call recorded more than one error";
+    }
+
+    void SetUp() override {
+        MobileGL::Initialize();
+        DrainPendingGlErrors();
+    }
+
+    void TearDown() override {
+        // Attribute a leaked error to the test that caused it instead of to whoever runs next.
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "test left an unconsumed GL error behind";
+    }
 };
 
 TEST_F(BufferTest, Binding) {
@@ -105,6 +129,53 @@ TEST_F(BufferTest, GenerateManyNames_NoPrematureCreation) {
     }
 }
 
+// GL 3.3 core 2.9 name lifecycle. The same three rules are asserted per object family (see the
+// texture/vertex-array/framebuffer/renderbuffer suites): a deleted or never-generated name is
+// INVALID_OPERATION to bind, deleting one is silent, and a generated-but-never-bound reservation
+// is still released so the name gets recycled.
+TEST_F(BufferTest, DeleteOfUnknownOrAlreadyDeletedBufferNameIsSilent) {
+    GLuint buffer = 0;
+    MG_Impl::GLImpl::GenBuffers(1, &buffer);
+    ASSERT_NE(buffer, 0u);
+    ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    MG_Impl::GLImpl::DeleteBuffers(1, &buffer);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    // Double delete, name 0 and a never-generated name must all be ignored without an error.
+    MG_Impl::GLImpl::DeleteBuffers(1, &buffer);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    const GLuint unknownNames[] = {0u, std::numeric_limits<GLuint>::max()};
+    MG_Impl::GLImpl::DeleteBuffers(2, unknownNames);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+}
+
+TEST_F(BufferTest, DeleteGeneratedButUnboundBufferNameReleasesReservationAndBindFails) {
+    GLuint buffer = 0;
+    MG_Impl::GLImpl::GenBuffers(1, &buffer);
+    ASSERT_NE(buffer, 0u);
+    ASSERT_TRUE(MG_State::pGLContext->ValidateBufferName(buffer));
+
+    MG_Impl::GLImpl::DeleteBuffers(1, &buffer);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+    EXPECT_FALSE(MG_State::pGLContext->ValidateBufferName(buffer));
+
+    MG_Impl::GLImpl::BindBuffer(GL_ARRAY_BUFFER, buffer);
+    ExpectSingleGlError(GL_INVALID_OPERATION);
+
+    GLuint recycled = 0;
+    MG_Impl::GLImpl::GenBuffers(1, &recycled);
+    EXPECT_EQ(recycled, buffer);
+}
+
+TEST_F(BufferTest, BindNeverGeneratedBufferNameIsInvalidOperation) {
+    // Not a small literal: other tests in this binary share the context and generate names in
+    // bulk, so a low number may well be a legitimately reserved name here.
+    MG_Impl::GLImpl::BindBuffer(GL_ARRAY_BUFFER, std::numeric_limits<GLuint>::max());
+    ExpectSingleGlError(GL_INVALID_OPERATION);
+}
+
 TEST_F(BufferTest, AcquireMemory) {
     auto& slot = MobileGL::MG_State::pGLContext->GetBufferBindingSlot(BufferTarget::Uniform);
     Vector<Uint> bufferNames;

MobileGL/MG_Test/Framebuffer/FramebufferTest.cpp

@@ -8,6 +8,8 @@
 
 #include <gtest/gtest.h>
 
+#include <limits>
+
 #include "Includes.h"
 #include "Init.h"
 #include <MG_Backend/BackendObjects.h>
@@ -78,8 +80,30 @@ namespace {
 
 class FramebufferTest : public ::testing::Test {
 protected:
+    // GL error flags are sticky per error code and the context outlives an individual test in this
+    // binary, so drain whatever an earlier test left pending - otherwise an error-code assertion
+    // here reads someone else's error. Bounded: one flag per code, so this cannot hang the suite.
+    static void DrainPendingGlErrors() {
+        for (Int drained = 0; drained < 16 && MG_Impl::GLImpl::GetError() != GL_NO_ERROR; ++drained) {
+        }
+    }
+
+    // The call under test must raise exactly the expected error and nothing more: a second pending
+    // error means one entry point queued several, which GetError() would hand out at an unrelated
+    // call site later on.
+    static void ExpectSingleGlError(GLenum expected) {
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), expected);
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "the call recorded more than one error";
+    }
+
+    void TearDown() override {
+        // Attribute a leaked error to the test that caused it instead of to whoever runs next.
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "test left an unconsumed GL error behind";
+    }
+
     void SetUp() override {
         MobileGL::Initialize();
+        DrainPendingGlErrors();
         const auto defaultFramebuffer = MG_State::pGLContext->GetFramebufferObject(0);
         ASSERT_NE(defaultFramebuffer, nullptr);
         MG_State::pGLContext->GetFramebufferBindingSlot(FramebufferTarget::Draw).Bind(defaultFramebuffer);
@@ -122,6 +146,83 @@ TEST_F(FramebufferTest, CreateFramebuffersCreatesObjectsImmediately) {
     EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
 }
 
+// GL 3.3 core 4.4.1/4.4.2 name lifecycle - mirrors the rules asserted for the other object
+// families: deleting an unknown name is silent, a released reservation is recycled, and binding
+// a dead name is INVALID_OPERATION.
+TEST_F(FramebufferTest, DeleteOfUnknownOrAlreadyDeletedFramebufferNameIsSilent) {
+    GLuint framebuffer = 0;
+    MG_Impl::GLImpl::GenFramebuffers(1, &framebuffer);
+    ASSERT_NE(framebuffer, 0u);
+    ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    MG_Impl::GLImpl::DeleteFramebuffers(1, &framebuffer);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    MG_Impl::GLImpl::DeleteFramebuffers(1, &framebuffer);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    // Not a small literal: other tests in this binary share the context and generate names in
+    // bulk, so a low number may well be a legitimately reserved name here.
+    const GLuint unknownNames[] = {0u, std::numeric_limits<GLuint>::max()};
+    MG_Impl::GLImpl::DeleteFramebuffers(2, unknownNames);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+}
+
+TEST_F(FramebufferTest, DeleteGeneratedButUnboundFramebufferNameReleasesReservationAndBindFails) {
+    GLuint framebuffer = 0;
+    MG_Impl::GLImpl::GenFramebuffers(1, &framebuffer);
+    ASSERT_NE(framebuffer, 0u);
+    ASSERT_TRUE(MG_State::pGLContext->ValidateFramebufferName(framebuffer));
+
+    MG_Impl::GLImpl::DeleteFramebuffers(1, &framebuffer);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+    EXPECT_FALSE(MG_State::pGLContext->ValidateFramebufferName(framebuffer));
+
+    MG_Impl::GLImpl::BindFramebuffer(GL_FRAMEBUFFER, framebuffer);
+    ExpectSingleGlError(GL_INVALID_OPERATION);
+
+    GLuint recycled = 0;
+    MG_Impl::GLImpl::GenFramebuffers(1, &recycled);
+    EXPECT_EQ(recycled, framebuffer);
+}
+
+TEST_F(FramebufferTest, DeleteOfUnknownOrAlreadyDeletedRenderbufferNameIsSilent) {
+    GLuint renderbuffer = 0;
+    MG_Impl::GLImpl::GenRenderbuffers(1, &renderbuffer);
+    ASSERT_NE(renderbuffer, 0u);
+    ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    MG_Impl::GLImpl::DeleteRenderbuffers(1, &renderbuffer);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    MG_Impl::GLImpl::DeleteRenderbuffers(1, &renderbuffer);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    // Not a small literal: other tests in this binary share the context and generate names in
+    // bulk, so a low number may well be a legitimately reserved name here.
+    const GLuint unknownNames[] = {0u, std::numeric_limits<GLuint>::max()};
+    MG_Impl::GLImpl::DeleteRenderbuffers(2, unknownNames);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+}
+
+TEST_F(FramebufferTest, DeleteGeneratedButUnboundRenderbufferNameReleasesReservationAndBindFails) {
+    GLuint renderbuffer = 0;
+    MG_Impl::GLImpl::GenRenderbuffers(1, &renderbuffer);
+    ASSERT_NE(renderbuffer, 0u);
+    ASSERT_TRUE(MG_State::pGLContext->ValidateRenderbufferName(renderbuffer));
+
+    MG_Impl::GLImpl::DeleteRenderbuffers(1, &renderbuffer);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+    EXPECT_FALSE(MG_State::pGLContext->ValidateRenderbufferName(renderbuffer));
+
+    MG_Impl::GLImpl::BindRenderbuffer(GL_RENDERBUFFER, renderbuffer);
+    ExpectSingleGlError(GL_INVALID_OPERATION);
+
+    GLuint recycled = 0;
+    MG_Impl::GLImpl::GenRenderbuffers(1, &recycled);
+    EXPECT_EQ(recycled, renderbuffer);
+}
+
 TEST_F(FramebufferTest, DefaultFramebufferIdentityTracksFramebufferNameZero) {
     const auto defaultFramebuffer = MG_State::pGLContext->GetFramebufferObject(0);
     ASSERT_NE(defaultFramebuffer, nullptr);

MobileGL/MG_Test/Texture/TextureTest.cpp

@@ -29,7 +29,32 @@ using namespace MobileGL;
 
 class TextureTest : public ::testing::Test {
 protected:
-    void SetUp() override { MobileGL::Initialize(); }
+    // GL error flags are sticky per error code and the context outlives an individual test in this
+    // binary, so anything an earlier test left pending would be handed to the next GetError() call -
+    // which silently turns error-code assertions into reads of someone else's error. Bounded because
+    // there is one flag per code; a runaway would otherwise hang the suite.
+    static void DrainPendingGlErrors() {
+        for (Int drained = 0; drained < 16 && MG_Impl::GLImpl::GetError() != GL_NO_ERROR; ++drained) {
+        }
+    }
+
+    // The call under test must raise exactly the expected error and nothing more: a second pending
+    // error means one entry point queued several (e.g. a shared validator firing before the
+    // specific check), which GetError() would hand out at unrelated call sites later on.
+    static void ExpectSingleGlError(GLenum expected) {
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), expected);
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "the call recorded more than one error";
+    }
+
+    void SetUp() override {
+        MobileGL::Initialize();
+        DrainPendingGlErrors();
+    }
+
+    void TearDown() override {
+        // Attribute a leaked error to the test that caused it instead of to whoever runs next.
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "test left an unconsumed GL error behind";
+    }
 };
 
 namespace {
@@ -258,6 +283,31 @@ TEST_F(TextureTest, SamplerMaxAnisotropyUsesTheSameStateAndValidationSemantics)
     EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
 }
 
+// GL 3.3 core 3.8.2: BindSampler rejects a never-generated or already-deleted name with
+// INVALID_OPERATION, while SamplerParameter* on the same name is INVALID_VALUE - the two paths
+// must not share one validator. Delete of an unknown name stays silent.
+TEST_F(TextureTest, BindSamplerRejectsUnknownNameWithInvalidOperationUnlikeSamplerParameter) {
+    GLuint sampler = 0;
+    MG_Impl::GLImpl::GenSamplers(1, &sampler);
+    ASSERT_NE(sampler, 0u);
+    MG_Impl::GLImpl::BindSampler(0, sampler);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    // Deleting is silent, twice over, and the name is dead afterwards.
+    MG_Impl::GLImpl::DeleteSamplers(1, &sampler);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+    MG_Impl::GLImpl::DeleteSamplers(1, &sampler);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    MG_Impl::GLImpl::BindSampler(0, sampler);
+    ExpectSingleGlError(GL_INVALID_OPERATION);
+
+    // Same dead name through SamplerParameter*: INVALID_VALUE, so the two paths cannot share one
+    // validator - and neither may queue the other's code alongside its own.
+    MG_Impl::GLImpl::SamplerParameteri(sampler, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    ExpectSingleGlError(GL_INVALID_VALUE);
+}
+
 TEST_F(TextureTest, GenThenBindCreatesObjectForUnsizedPackedBgraSubImageUpload) {
     GLuint texture = 0;
     MG_Impl::GLImpl::GenTextures(1, &texture);
@@ -307,10 +357,12 @@ TEST_F(TextureTest, DeleteGeneratedButUnboundNameReleasesReservationAndBindFails
     EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
     EXPECT_FALSE(MG_State::pGLContext->ValidateTextureName(texture));
     EXPECT_FALSE(MG_State::pGLContext->ValidateTextureObject(texture));
+    // IsTexture answers about a dead name without raising anything (GL 3.3 core 6.1.4).
     EXPECT_EQ(MG_Impl::GLImpl::IsTexture(texture), GL_FALSE);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
 
     MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, texture);
-    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
+    ExpectSingleGlError(GL_INVALID_OPERATION);
     EXPECT_FALSE(MG_State::pGLContext->ValidateTextureObject(texture));
 
     // The freed reservation is recycled (the generator's free list is LIFO, so the very same
@@ -340,7 +392,7 @@ TEST_F(TextureTest, DeleteInstantiatedTextureInvalidatesNameUntilRegenerated) {
     ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
 
     MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, textures[0]);
-    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
+    ExpectSingleGlError(GL_INVALID_OPERATION);
     EXPECT_EQ(MG_State::pGLContext->GetTextureUnitObject(0)
                   .GetBindingSlot(TextureTarget::Texture2D)
                   .GetBoundObject(),
@@ -359,7 +411,7 @@ TEST_F(TextureTest, DeleteUnknownNamesIsSilentButBindUnknownNameIsInvalid) {
     EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
 
     MG_Impl::GLImpl::BindTexture(GL_TEXTURE_2D, unknownNames[1]);
-    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_INVALID_OPERATION);
+    ExpectSingleGlError(GL_INVALID_OPERATION);
     EXPECT_EQ(MG_State::pGLContext->GetTextureUnitObject(0)
                   .GetBindingSlot(TextureTarget::Texture2D)
                   .GetBoundObject(),

MobileGL/MG_Test/VertexArray/VertexArrayTest.cpp

@@ -8,6 +8,8 @@
 
 #include <gtest/gtest.h>
 
+#include <limits>
+
 #include "Includes.h"
 #include "Init.h"
 
@@ -35,9 +37,31 @@ protected:
 
         return vbo;
     }
-    void SetUp() override { MobileGL::Initialize(); }
+    // GL error flags are sticky per error code and the context outlives an individual test in this
+    // binary, so drain whatever an earlier test left pending - otherwise an error-code assertion
+    // here reads someone else's error. Bounded: one flag per code, so this cannot hang the suite.
+    static void DrainPendingGlErrors() {
+        for (Int drained = 0; drained < 16 && MG_Impl::GLImpl::GetError() != GL_NO_ERROR; ++drained) {
+        }
+    }
 
-    void TearDown() override {}
+    // The call under test must raise exactly the expected error and nothing more: a second pending
+    // error means one entry point queued several, which GetError() would hand out at an unrelated
+    // call site later on.
+    static void ExpectSingleGlError(GLenum expected) {
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), expected);
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "the call recorded more than one error";
+    }
+
+    void SetUp() override {
+        MobileGL::Initialize();
+        DrainPendingGlErrors();
+    }
+
+    void TearDown() override {
+        // Attribute a leaked error to the test that caused it instead of to whoever runs next.
+        EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR) << "test left an unconsumed GL error behind";
+    }
 };
 
 TEST_F(VertexArrayTest, GenerateAndBindVAO) {
@@ -57,6 +81,46 @@ TEST_F(VertexArrayTest, GenerateAndBindVAO) {
     // Do not detect if it supports default VAO
 }
 
+// GL 3.3 core 2.10 name lifecycle - the same three rules the other object families assert:
+// deleting an unknown name is silent, a released reservation is recycled, and binding a dead
+// name is INVALID_OPERATION.
+TEST_F(VertexArrayTest, DeleteOfUnknownOrAlreadyDeletedVertexArrayNameIsSilent) {
+    GLuint vao = 0;
+    MG_Impl::GLImpl::GenVertexArrays(1, &vao);
+    ASSERT_NE(vao, 0u);
+    ASSERT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    MG_Impl::GLImpl::DeleteVertexArrays(1, &vao);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    MG_Impl::GLImpl::DeleteVertexArrays(1, &vao);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+
+    // Not a small literal: other tests in this binary share the context and generate names in
+    // bulk, so a low number may well be a legitimately reserved name here.
+    const GLuint unknownNames[] = {0u, std::numeric_limits<GLuint>::max()};
+    MG_Impl::GLImpl::DeleteVertexArrays(2, unknownNames);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+}
+
+TEST_F(VertexArrayTest, DeleteGeneratedButUnboundVertexArrayNameReleasesReservationAndBindFails) {
+    GLuint vao = 0;
+    MG_Impl::GLImpl::GenVertexArrays(1, &vao);
+    ASSERT_NE(vao, 0u);
+    ASSERT_TRUE(MG_State::pGLContext->ValidateVertexArrayName(vao));
+
+    MG_Impl::GLImpl::DeleteVertexArrays(1, &vao);
+    EXPECT_EQ(MG_Impl::GLImpl::GetError(), GL_NO_ERROR);
+    EXPECT_FALSE(MG_State::pGLContext->ValidateVertexArrayName(vao));
+
+    MG_Impl::GLImpl::BindVertexArray(vao);
+    ExpectSingleGlError(GL_INVALID_OPERATION);
+
+    GLuint recycled = 0;
+    MG_Impl::GLImpl::GenVertexArrays(1, &recycled);
+    EXPECT_EQ(recycled, vao);
+}
+
 TEST_F(VertexArrayTest, VertexAttributeSetup) {
     Vector<Uint> vaoNames;
     MobileGL::MG_State::pGLContext->GenVertexArrayNames(1, vaoNames);
@@ -264,7 +328,9 @@ TEST_F(VertexArrayTest, VertexBindingIndexIsBoundedByTheAdvertisedAttribLimit) {
 
     const GLuint outOfRange = MG_Impl::GLImpl::VertexArrayImpl::GetMaxVertexAttribs();
     MG_Impl::GLImpl::VertexAttribBinding(0, outOfRange);
-    EXPECT_TRUE(MG_State::pGLContext->HasGLError());
+    // Asserting the exact code (rather than just "some error") also consumes it, so the next test
+    // does not inherit it - GL error flags are sticky and this context is shared.
+    ExpectSingleGlError(GL_INVALID_VALUE);
 }
 
 // The default attribute -> binding-point mapping is the identity. It used to be a 16-element literal