From d4c6ff434c5c7aa66bc7c28e5ce336d2a694bfcc Mon Sep 17 00:00:00 2001 From: Nikolai Nechaev Date: Tue, 28 Apr 2026 19:17:12 +0900 Subject: hyprtester: minor refactoring/restructure (#14154) * hyprtester: Minor misc fixes - Use `std::filesystem::path` instead of `std::string` in some places; - Use `std::ranges::sort`, as suggested by clang-tidy; - Reset colors at the end of every log line; - Give more specific error messages in main. * hyprtester: Fix a bug in `hyprlandAlive()` Successful standard library calls are permitted to (and do) modify the value of `errno` arbitrarily. Checking the value of `errno` only makes sense when the library function returns an error. So, the correct condition of a successful signal delivery is that the call to `kill` returns 0. Btw, given the possible other error values of `kill(2)`, should we even consider `errno` at all? * hyprtester: Restructure main Split main's `main()` into multiple functions, separate responsibilities more cleanly. Add default values for cmd-line options to the help message. * hyprtester: Remove redundant path canonicalization * hyprtester: Run unified clean up code before every test * tests: Use RAII for clients in client tests Turn clients in client tests into classes. Constructors initialize, destructors deinitialize. Not only is it better design when entities (clients) are modeled as classes, keeping all the methods connected, this also ensures the tests perform their custom cleanup. This will be especially important when we introduce early termination on failure into the tests. * hyprtester: Rewrite the testing framework! - Implement a new test definition system used via the `TEST_CASE` and `SUBTEST` macros. - Adds `ASSERT*` and reimplements `EXPECT*` macros. The former now terminate the test immediately. Also add some new macros, such as `FAIL_TEST`. - Move existing tests to the new system. For trivial cases, break existing test functions down to multiple test cases. - For non-trivial cases and a few other possible improvements, leave TODOs. * tests: Use `EXPECT*` instead of `ASSERT*` in some places `ASSERT*` macros terminate the test as soon as they fail. They are appropriate to use when, if something fails, the rest of the test does not make sense and more error messages from failing checks will just pollute the output. `EXPECT*` macros mark the test as failed but its execution continues. It makes sense to use them when error messages from the following checks in the test may be useful for someone who will be reading the log. I changed some uses of `ASSERT*` in main tests to `EXPECT*`. But I was not reading tests very carefully, so maybe I was wrong in some places. My rule of thumb was: - Checks that ensure that a kitty opens / a certain set/number of windows exists are fatal; - A series of checks, especially on the same string returned by `Hyprland`, are non-fatal; - Checks in tests with tightly coupled steps (i.e., do this, check, do this with the result of the previous step, check, etc) are fatal. --- hyprtester/src/tests/clients/child-window.cpp | 113 +++++++++++++------------- 1 file changed, 58 insertions(+), 55 deletions(-) (limited to 'hyprtester/src/tests/clients/child-window.cpp') diff --git a/hyprtester/src/tests/clients/child-window.cpp b/hyprtester/src/tests/clients/child-window.cpp index 40118c7c..f97ffde1 100644 --- a/hyprtester/src/tests/clients/child-window.cpp +++ b/hyprtester/src/tests/clients/child-window.cpp @@ -1,4 +1,3 @@ -#include "../../shared.hpp" #include "../../hyprctlCompat.hpp" #include "../shared.hpp" #include "tests.hpp" @@ -7,6 +6,7 @@ #include #include +#include #include #include #include @@ -17,15 +17,6 @@ using namespace Hyprutils::Memory; #define SP CSharedPointer -struct SClient { - SP proc; - std::array readBuf; - CFileDescriptor readFd, writeFd; - struct pollfd fds; -}; - -static int ret = 0; - static bool waitForWindow(SP proc, int windowsBefore) { int counter = 0; while (Tests::processAlive(proc->pid()) && Tests::windowCount() == windowsBefore) { @@ -40,56 +31,69 @@ static bool waitForWindow(SP proc, int windowsBefore) { return Tests::processAlive(proc->pid()); } -static bool startClient(SClient& client) { +namespace { + class CClient { + SP proc; + std::array readBuf; + CFileDescriptor readFd, writeFd; + struct pollfd fds; + + public: + CClient(); + ~CClient(); + bool createChild(); + }; +} + +CClient::CClient() { NLog::log("{}Attempting to start child-window client", Colors::YELLOW); - client.proc = makeShared(binaryDir + "/child-window", std::vector{}); + this->proc = makeShared(binaryDir + "/child-window", std::vector{}); - client.proc->addEnv("WAYLAND_DISPLAY", WLDISPLAY); + this->proc->addEnv("WAYLAND_DISPLAY", WLDISPLAY); int procInPipeFd[2], procOutPipeFd[2]; if (pipe(procInPipeFd) != 0 || pipe(procOutPipeFd) != 0) { NLog::log("{}Unable to open pipe to client", Colors::RED); - return false; + throw std::exception(); } - client.writeFd = CFileDescriptor(procInPipeFd[1]); - client.proc->setStdinFD(procInPipeFd[0]); + this->writeFd = CFileDescriptor(procInPipeFd[1]); + this->proc->setStdinFD(procInPipeFd[0]); - client.readFd = CFileDescriptor(procOutPipeFd[0]); - client.proc->setStdoutFD(procOutPipeFd[1]); + this->readFd = CFileDescriptor(procOutPipeFd[0]); + this->proc->setStdoutFD(procOutPipeFd[1]); - if (!client.proc->runAsync()) { + if (!this->proc->runAsync()) { NLog::log("{}Failed to run client", Colors::RED); - return false; + throw std::exception(); } close(procInPipeFd[0]); close(procOutPipeFd[1]); - if (!waitForWindow(client.proc, Tests::windowCount())) { + if (!waitForWindow(this->proc, Tests::windowCount())) { NLog::log("{}Window took too long to open", Colors::RED); - return false; + throw std::exception(); } NLog::log("{}Started child-window client", Colors::YELLOW); - return true; } -static void stopClient(SClient& client) { +CClient::~CClient() { std::string cmd = "exit\n"; - write(client.writeFd.get(), cmd.c_str(), cmd.length()); + write(this->writeFd.get(), cmd.c_str(), cmd.length()); - kill(client.proc->pid(), SIGKILL); - client.proc.reset(); + kill(this->proc->pid(), SIGKILL); + this->proc.reset(); } -static bool createChild(SClient& client) { +bool CClient::createChild() { std::string cmd = "toplevel\n"; - if ((size_t)write(client.writeFd.get(), cmd.c_str(), cmd.length()) != cmd.length()) + if ((size_t)write(this->writeFd.get(), cmd.c_str(), cmd.length()) != cmd.length()) return false; - if (!waitForWindow(client.proc, Tests::windowCount())) + if (!waitForWindow(this->proc, Tests::windowCount())) NLog::log("{}Child window took too long to open", Colors::RED); if (getFromSocket("/dispatch hl.dsp.focus({ window = 'class:child-test-child' })") != "ok") { @@ -100,19 +104,21 @@ static bool createChild(SClient& client) { return true; } -static bool test() { - SClient client; +TEST_CASE(childWindow) { + { + std::optional client; + try { + client.emplace(); + } catch (...) { FAIL_TEST("Couldn't start the client"); } - if (!startClient(client)) - return false; - OK(getFromSocket("/dispatch hl.dsp.window.float({ action = 'set', window = 'class:child-test-parent' })")); - OK(getFromSocket("/dispatch hl.dsp.window.pin({ action = 'set', window = 'class:child-test-parent' })")); + OK(getFromSocket("/dispatch hl.dsp.window.float({ action = 'set', window = 'class:child-test-parent' })")); + OK(getFromSocket("/dispatch hl.dsp.window.pin({ action = 'set', window = 'class:child-test-parent' })")); - createChild(client); - EXPECT(Tests::windowCount(), 2) - EXPECT_COUNT_STRING(getFromSocket("/clients"), "pinned: 1", 2); + client->createChild(); + EXPECT(Tests::windowCount(), 2); + EXPECT_COUNT_STRING(getFromSocket("/clients"), "pinned: 1", 2); + } - stopClient(client); NLog::log("{}Reloading config", Colors::YELLOW); OK(getFromSocket("/reload")); Tests::killAllWindows(); @@ -122,30 +128,27 @@ static bool test() { NLog::log("{}Test child windows are not auto-grouped", Colors::GREEN); auto kitty = Tests::spawnKitty(); if (!kitty) { - NLog::log("{}Error: kitty did not spawn", Colors::RED); - return false; + FAIL_TEST("Couldn't spawn kitty"); } // create group and enable auto-grouping OK(getFromSocket("/dispatch hl.dsp.group.toggle()")); OK(getFromSocket("/eval hl.config({ group = { auto_group = true } })")); - SClient client2; - if (!startClient(client2)) - return false; + { + std::optional client2; + try { + client2.emplace(); + } catch (...) { FAIL_TEST("Couldn't start the client"); } - EXPECT(Tests::windowCount(), 2); - createChild(client2); - EXPECT(Tests::windowCount(), 3); + EXPECT(Tests::windowCount(), 2); + client2->createChild(); + EXPECT(Tests::windowCount(), 3); - // child has set_parent so shouldBeFloated returns true, it should not be auto-grouped - EXPECT_COUNT_STRING(getFromSocket("/clients"), "grouped: 0", 1); + // child has set_parent so shouldBeFloated returns true, it should not be auto-grouped + EXPECT_COUNT_STRING(getFromSocket("/clients"), "grouped: 0", 1); + } - stopClient(client2); Tests::killAllWindows(); EXPECT(Tests::windowCount(), 0); - - return !ret; } - -REGISTER_CLIENT_TEST_FN(test); -- cgit v1.2.3