Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add basic tests for SleeperThread #2608

Closed

Conversation

taminob
Copy link
Contributor

@taminob taminob commented Oct 24, 2023

I wrote these after failing to construct test cases for #2601.
Unfortunately, I couldn't think of tests without relying on some timing, but I hope that I chose the times so that the tests will work on all (relevant) systems.

For this, I restructured the build system to allow for easier addition to the tests by compiling all sources of waybar (except main.cpp) as a static library.
This library is then linked against main.cpp for the main executable and against the test files for the test executable.

@taminob taminob marked this pull request as draft October 24, 2023 16:47
@taminob taminob force-pushed the feature/add-tests-for-sleeper-thread branch 9 times, most recently from d2e9c5f to e5cd237 Compare January 29, 2024 22:43
This allows compilation of "include/util/command.hpp" without the main
source file.
This library can be linked to the main executable and the test
executable.
@taminob taminob force-pushed the feature/add-tests-for-sleeper-thread branch from e5cd237 to a0db413 Compare January 29, 2024 22:45
@taminob
Copy link
Contributor Author

taminob commented Feb 4, 2024

I don't think anymore that these SleeperThread tests are worth merging since they are too brittle and depend on specific timing to succeed - I tried to improve them, but wasn't really able to achieve guaranteed success without any timing dependency. Thus, I'll close the PR.

@Alexays if you think the other changes to allow linking against a static waybar lib in the tests are worth adding, feel free to ping me and I can open another PR for that.

@taminob taminob closed this Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant