fix(test): process_fuzz_input returning EXIT_SUCCESS on non-zero harness returns

test(surface): add & fix unit tests

fix(surface): bugs

refactor(surface): minor refactors & some edge-case handling

modules/mirror/private/entrypoint/mirror.cpp

@@ -52,7 +52,7 @@ public:
 		m_window = m_editor_registry->create_entity("Editor Window");
 		m_window.add_component<SurfaceComponent>(SurfaceComponent::CreateInfo {
 		    .title = "Editor Window",
-		    .size = { 800u, 600u },
+		    .resolution = { 800u, 600u },
 		    .vsync = true,
 		    .visible = true,
 		});

modules/surface/CMakeLists.txt

@@ -13,4 +13,4 @@ target_link_libraries(surface PUBLIC
 )
 
 add_test_module(surface system.test.cpp)
-target_link_libraries(surface_tests PRIVATE glfw)
+add_fuzz_module(surface system.fuzz.cpp)

modules/surface/private/linux/system.cpp

@@ -5,6 +5,11 @@
 
 namespace lt::surface {
 
+void glfw_error_callbac(int32_t code, const char *description)
+{
+	log_err("GLFW ERROR: {} -> {}", code, description);
+}
+
 void handle_event(GLFWwindow *window, const SurfaceComponent::Event &event)
 {
 	auto &callbacks = *static_cast<std::vector<SurfaceComponent::EventCallback> *>(
@@ -92,9 +97,15 @@ void bind_glfw_events(GLFWwindow *handle)
 	});
 }
 
+void init_glfw() {};
+
 System::System(Ref<ecs::Registry> registry): m_registry(std::move(registry))
 {
+	glfwSetErrorCallback(&glfw_error_callbac);
+	ensure(glfwInit(), "Failed to initialize 'glfw'");
+
 	ensure(m_registry, "Failed to initialize surface system: null registry");
+
 	ensure(
 	    m_registry->view<SurfaceComponent>().size() == 0,
 	    "Failed to initialize surface system: registry has surface component(s)"
@@ -129,7 +140,6 @@ System::~System()
 
 
 	m_registry->view<SurfaceComponent>().each([&](const entt::entity entity, SurfaceComponent &) {
-		std::cout << "REMOVED SURFACE COMPONENT ON DESTRUCTION" << std::endl;
 		m_registry->get_entt_registry().remove<SurfaceComponent>(entity);
 	});
 
@@ -138,28 +148,33 @@ System::~System()
 
 void System::on_surface_construct(entt::registry &registry, entt::entity entity)
 {
-	ensure(glfwInit(), "Failed to initialize 'glfw'");
+	try
+	{
+		auto &surface = registry.get<SurfaceComponent>(entity);
+		ensure_component_sanity(surface);
 
-	glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 4);
-	glfwWindowHint(GLFW_CONTEXT_VERSION_MINOR, 5);
-	glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE);
-	// glfwWindowHint(GLFW_VISIBLE, GLFW_FALSE);
+		glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 4);
+		glfwWindowHint(GLFW_CONTEXT_VERSION_MINOR, 5);
+		glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE);
 
-	auto &surface = registry.get<SurfaceComponent>(entity);
-	auto [width, height] = surface.get_size();
+		surface.m_glfw_handle = glfwCreateWindow(
+		    static_cast<int>(surface.get_resolution().x),
+		    static_cast<int>(surface.get_resolution().y),
+		    surface.get_title().begin(),
+		    nullptr,
+		    nullptr
+		);
+		ensure(surface.m_glfw_handle, "Failed to create 'GLFWwindow'");
 
-	surface.m_glfw_handle = glfwCreateWindow(
-	    static_cast<int32_t>(width),
-	    static_cast<int32_t>(height),
-	    surface.get_title().begin(),
-	    nullptr,
-	    nullptr
-	);
-	ensure(surface.m_glfw_handle, "Failed to create 'GLFWwindow'");
-
-	glfwSetWindowUserPointer(surface.m_glfw_handle, &surface.m_event_callbacks);
-	surface.m_native_handle = glfwGetX11Window(surface.m_glfw_handle);
-	bind_glfw_events(surface.m_glfw_handle);
+		glfwSetWindowUserPointer(surface.m_glfw_handle, &surface.m_event_callbacks);
+		surface.m_native_handle = glfwGetX11Window(surface.m_glfw_handle);
+		bind_glfw_events(surface.m_glfw_handle);
+	}
+	catch (...)
+	{
+		registry.remove<SurfaceComponent>(entity);
+		throw;
+	}
 }
 
 void System::on_surface_update(entt::registry &registry, entt::entity entity)
@@ -171,7 +186,11 @@ void System::on_surface_update(entt::registry &registry, entt::entity entity)
 void System::on_surface_destroy(entt::registry &registry, entt::entity entity)
 {
 	auto &surface = registry.get<SurfaceComponent>(entity);
-	glfwDestroyWindow(surface.m_glfw_handle);
+
+	if (surface.m_glfw_handle)
+	{
+		glfwDestroyWindow(surface.m_glfw_handle);
+	}
 }
 
 void System::set_title(ecs::Entity entity, std::string_view new_title)
@@ -179,11 +198,15 @@ void System::set_title(ecs::Entity entity, std::string_view new_title)
 	auto &surface = entity.get_component<SurfaceComponent>();
 
 	surface.m_title = new_title;
-	glfwSetWindowTitle(surface.m_glfw_handle, surface.m_title.begin());
+	glfwSetWindowTitle(surface.m_glfw_handle, surface.m_title.c_str());
 }
 
 auto System::tick() -> bool
 {
+	m_registry->view<SurfaceComponent>().each([](SurfaceComponent &surface) {
+		glfwSwapBuffers(surface.m_glfw_handle);
+	});
+
 	glfwPollEvents();
 	return false;
 }
@@ -191,7 +214,7 @@ auto System::tick() -> bool
 void System::set_size(ecs::Entity surface_entity, const math::uvec2 &new_size)
 {
 	auto &surface = surface_entity.get_component<SurfaceComponent>();
-	surface.m_size = new_size;
+	surface.m_resolution = new_size;
 
 	glfwSetWindowSize(
 	    surface.m_glfw_handle,
@@ -232,6 +255,36 @@ void System::add_event_listener(
 	surface.m_event_callbacks.emplace_back(std::move(callback));
 }
 
+void System::ensure_component_sanity(const SurfaceComponent &component)
+{
+	auto [width, height] = component.get_resolution();
+
+	ensure(width != 0u, "Received bad values for surface component: width({}) == 0", width);
+
+	ensure(height != 0u, "Received bad values for surface component: height({}) == 0", height);
+
+	ensure(
+	    width < SurfaceComponent::max_dimension,
+	    "Received bad values for surface component: width({}) > max_dimension({})",
+	    width,
+	    SurfaceComponent::max_dimension
+	);
+
+	ensure(
+	    height < SurfaceComponent::max_dimension,
+	    "Received bad values for surface component: height({}) > max_dimension({})",
+	    height,
+	    SurfaceComponent::max_dimension
+	);
+
+	ensure(
+	    component.get_title().size() < SurfaceComponent::max_title_length,
+	    "Received bad values for surface component: title.size({}) > max_title_length({})",
+	    component.get_title().size(),
+	    SurfaceComponent::max_title_length
+	);
+}
+
 } // namespace lt::surface
 
 namespace lt {

modules/surface/private/system.fuzz.cpp

@@ -0,0 +1,103 @@
+#include <ecs/scene.hpp>
+#include <surface/system.hpp>
+#include <test/fuzz.hpp>
+#include <test/test.hpp>
+
+namespace lt::surface {
+
+enum class Action : uint8_t
+{
+	create_entity,
+
+	create_surface_component,
+
+	destroy_surface_component,
+
+	tick,
+};
+
+void create_surface_component(test::FuzzDataProvider &provider, ecs::Registry &registry)
+{
+	const auto length = std::min(provider.consume<uint32_t>().value_or(16), 255u);
+	const auto title = provider.consume_string(length).value_or("");
+
+	const auto resolution = math::uvec2 {
+		provider.consume<uint32_t>().value_or({ 32 }),
+		provider.consume<uint32_t>().value_or({ 64 }),
+	};
+	const auto visible = provider.consume<bool>().value_or(false);
+	const auto vsync = provider.consume<bool>().value_or(false);
+
+	try
+	{
+		registry.create_entity("").add_component<surface::SurfaceComponent>(
+		    surface::SurfaceComponent::CreateInfo {
+		        .title = std::move(title),
+		        .resolution = resolution,
+		        .vsync = vsync,
+		        .visible = visible,
+		    }
+		);
+	}
+	catch (const std::exception &exp)
+	{
+		std::ignore = exp;
+	}
+}
+
+void remove_surface_component(ecs::Registry &registry)
+{
+	const auto view = registry.get_entt_registry().view<SurfaceComponent>();
+
+	if (!view->empty())
+	{
+		registry.get_entt_registry().remove<SurfaceComponent>(*view.begin());
+	}
+}
+
+void check_invariants()
+{
+}
+
+test::FuzzHarness harness = [](const uint8_t *data, size_t size) {
+	auto provider = test::FuzzDataProvider { data, size };
+
+	auto registry = create_ref<ecs::Registry>();
+	auto system = surface::System { registry };
+
+	while (auto action = provider.consume<uint8_t>())
+	{
+		switch (static_cast<Action>(action.value()))
+		{
+		case Action::create_entity:
+		{
+			const auto length = std::min(provider.consume<uint32_t>().value_or(16), 255u);
+			const auto tag = provider.consume_string(length).value_or("");
+			registry->create_entity(tag);
+
+			break;
+		}
+		case Action::create_surface_component:
+		{
+			create_surface_component(provider, *registry);
+			break;
+		}
+		case Action::destroy_surface_component:
+		{
+			remove_surface_component(*registry);
+			break;
+		}
+		case Action::tick:
+		{
+			system.tick();
+			break;
+		}
+		}
+
+		check_invariants();
+	}
+
+	return 0;
+};
+
+} // namespace lt::surface

modules/surface/private/system.test.cpp

@@ -26,22 +26,24 @@ public:
 		return m_registry;
 	}
 
-	auto add_surface_component() -> SurfaceComponent &
+	auto add_surface_component(
+	    SurfaceComponent::CreateInfo info = SurfaceComponent::CreateInfo {
+	        .title = title,
+	        .resolution = { width, height },
+	        .vsync = vsync,
+	        .visible = visible,
+	    }
+	) -> SurfaceComponent &
 	{
 		auto entity = m_registry->create_entity("");
-		return entity.add_component<SurfaceComponent>(SurfaceComponent::CreateInfo {
-		    .title = title,
-		    .size = { width, height },
-		    .vsync = vsync,
-		    .visible = visible,
-		});
+		return entity.add_component<SurfaceComponent>(info);
 	}
 
 	void check_values(const SurfaceComponent &component)
 	{
 		expect_ne(std::get<SurfaceComponent::X11NativeHandle>(component.get_native_handle()), 0);
-		expect_eq(component.get_size().x, width);
-		expect_eq(component.get_size().y, height);
+		expect_eq(component.get_resolution().x, width);
+		expect_eq(component.get_resolution().y, height);
 		expect_eq(component.get_title(), title);
 		expect_eq(component.is_vsync(), vsync);
 		expect_eq(component.is_visible(), visible);
@@ -57,9 +59,11 @@ Suite raii = [] {
 		ignore = System { fixture.registry() };
 	};
 
-	Case { "many won't throw" } = [] {
+	Case { "many won't freeze/throw" } = [] {
 		auto fixture = Fixture {};
-		for (auto idx : std::views::iota(0, 100'001))
+
+		/* range is small since glfw init/terminate is slow. */
+		for (auto idx : std::views::iota(0, 100))
 		{
 			ignore = System { fixture.registry() };
 		}
@@ -78,6 +82,17 @@ Suite raii = [] {
 		auto system = System { fixture.registry() };
 		expect_eq(fixture.registry()->view<SurfaceComponent>()->size(), 0);
 	};
+
+	Case { "post destruct has correct state" } = [] {
+		auto fixture = Fixture {};
+		auto system = create_scope<System>(fixture.registry());
+
+		fixture.add_surface_component();
+		expect_eq(fixture.registry()->view<SurfaceComponent>()->size(), 1);
+
+		system.reset();
+		expect_eq(fixture.registry()->view<SurfaceComponent>()->size(), 0);
+	};
 };
 
 Suite system_events = [] {
@@ -109,6 +124,41 @@ Suite registry_events = [] {
 		fixture.check_values(component);
 	};
 
+	Case { "unhappy on_construct<SurfaceComponent> throws" } = [] {
+		auto fixture = Fixture {};
+		auto system = System { fixture.registry() };
+
+		expect_throw([&] { fixture.add_surface_component({ .resolution = { width, 0 } }); });
+
+		expect_throw([&] { fixture.add_surface_component({ .resolution = { 0, height } }); });
+
+		expect_throw([&] {
+			fixture.add_surface_component(
+			    { .title = "", .resolution = { SurfaceComponent::max_dimension + 1, height } }
+			);
+		});
+
+		expect_throw([&] {
+			fixture.add_surface_component(
+			    { .title = "", .resolution = { width, SurfaceComponent::max_dimension + 1 } }
+			);
+		});
+
+		auto big_str = std::string {};
+		big_str.resize(SurfaceComponent::max_title_length + 1);
+		expect_throw([&] {
+			fixture.add_surface_component({ .title = big_str, .resolution = { width, height } });
+		});
+	};
+
+	Case { "unhappy on_construct<SurfaceComponent> removes component" } = [] {
+		auto fixture = Fixture {};
+		auto system = System { fixture.registry() };
+
+		expect_throw([&] { fixture.add_surface_component({ .resolution = { width, 0 } }); });
+		expect_eq(fixture.registry()->view<SurfaceComponent>().size(), 0);
+	};
+
 	Case { "on_destrroy<SurfaceComponent> cleans up component" } = [] {
 		auto fixture = Fixture {};
 		auto system = create_scope<System>(fixture.registry());
@@ -137,7 +187,7 @@ Suite tick = [] {
 	};
 
 	Case { "ticking on chaotic registry won't throw" } = [] {
-	}
+	};
 };
 
 Suite property_setters = [] {
@@ -146,6 +196,3 @@ Suite property_setters = [] {
 
 Suite listeners = [] {
 };
-
-Suite fuzzy = [] {
-};

modules/surface/public/components.hpp

@@ -47,11 +47,15 @@ public:
 
 	using NativeHandle = std::variant<WindowsNativeHandle, X11NativeHandle>;
 
+	static constexpr auto max_dimension = 4096;
+
+	static constexpr auto max_title_length = 256;
+
 	struct CreateInfo
 	{
 		std::string_view title;
 
-		math::uvec2 size;
+		math::uvec2 resolution;
 
 		bool vsync;
 
@@ -60,20 +64,20 @@ public:
 
 	SurfaceComponent(const CreateInfo &info)
 	    : m_title(info.title)
-	    , m_size(info.size)
+	    , m_resolution(info.resolution)
 	    , m_vsync(info.vsync)
 	    , m_visible(info.visible)
 	{
 	}
 
-	[[nodiscard]] auto get_title() const -> const std::string_view &
+	[[nodiscard]] auto get_title() const -> std::string_view
 	{
 		return m_title;
 	}
 
-	[[nodiscard]] auto get_size() const -> const math::uvec2 &
+	[[nodiscard]] auto get_resolution() const -> const math::uvec2 &
 	{
-		return m_size;
+		return m_resolution;
 	}
 
 	[[nodiscard]] auto is_vsync() const -> bool
@@ -97,9 +101,9 @@ private:
 		return m_glfw_handle;
 	}
 
-	std::string_view m_title;
+	std::string m_title;
 
-	math::uvec2 m_size;
+	math::uvec2 m_resolution;
 
 	bool m_vsync;
 

modules/surface/public/system.hpp

@@ -32,7 +32,7 @@ public:
 
 	auto tick() -> bool override;
 
-	void set_title(ecs::Entity surface_entity, std::string_view new_title);
+	static void set_title(ecs::Entity surface_entity, std::string_view new_title);
 
 	void set_size(ecs::Entity surface_entity, const math::uvec2 &new_size);
 
@@ -49,6 +49,8 @@ private:
 
 	void on_surface_destroy(entt::registry &registry, entt::entity entity);
 
+	void ensure_component_sanity(const SurfaceComponent &component);
+
 	Ref<ecs::Registry> m_registry;
 };
 

modules/test/private/fuzz.cpp

@@ -1,11 +1,11 @@
 #include <test/test.hpp>
 
 namespace lt::test {
+
 auto process_fuzz_input(const uint8_t *data, size_t size) -> int32_t
 try
 {
-	details::Registry::process_fuzz_input(data, size);
-	return EXIT_SUCCESS;
+	return details::Registry::process_fuzz_input(data, size);
 }
 catch (const std::exception &exp)
 {