Make tests more platform agnostic #4650
Conversation
tests/src/InputCompletion.cpp
Outdated
| ASSERT_EQ(completion[0].displayName, "FeelsBirthdayMan"); | ||
| ASSERT_EQ(completion[1].displayName, "FeelsBadMan"); | ||
| // all these matches are BTTV global emotes | ||
| std::sort(completion.begin(), completion.end(), cmpCompletion); |
There was a problem hiding this comment.
I'm hesitant about this change - I'd rather leave this change out as part of the platform-agnostic PR & discuss it in a separate PR/issue.
Most likely I'd like to see us just change the EmoteMap to be consistently ordered instead
There was a problem hiding this comment.
I reverted the change, but added a note for future visitors.
Most likely I'd like to see us just change the EmoteMap to be consistently ordered instead
Not sure if it's that easy. It should probably be benchmarked to see if it makes a meaningful difference, since working with emotes and checking for emotes in a word is an integral part.
pajlada
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -66,7 +67,8 @@ class MockApplication : mock::EmptyApplication | |||
| static void BM_HighlightTest(benchmark::State &state) | |||
| { | |||
| MockApplication mockApplication; | |||
| Settings settings("/tmp/c2-mock"); | |||
| QTemporaryDir settingsDir; | |||
| Settings settings(settingsDir.path()); | |||
There was a problem hiding this comment.
warning: variable 'settings' of type 'chatterino::Settings' can be declared 'const' [misc-const-correctness]
| Settings settings(settingsDir.path()); | |
| Settings const settings(settingsDir.path()); |
| QTemporaryDir settingsDir; | ||
| settingsDir.setAutoRemove(false); // we'll remove it manually | ||
| qDebug() << "Settings directory:" << settingsDir.path(); | ||
| chatterino::Settings settings(settingsDir.path()); |
There was a problem hiding this comment.
warning: variable 'settings' of type 'chatterino::Settings' can be declared 'const' [misc-const-correctness]
| chatterino::Settings settings(settingsDir.path()); | |
| chatterino::Settings const settings(settingsDir.path()); |
Description
/tmpdirectory the same way, e.g. Linux has it. So instead, the tests are now usingQTemporaryDir- which is cross-platform.CHATTERINO_TEST_LOCAL_HTTPBIN=Off).std::unordered_mapthan the one used when writing the input completion tests. Sinceunordered_mapdoesn't guarantee an ordering (the name explicitly saysunordered), one can't rely on values being in order in such a map. This map is the underlying data-type for anEmoteMap. For the input completions, this is a bit more tricky, since they do guarantee some amount of ordering (regarding the emote/emoji provider). So, for an emote provider, the completions are now sorted.Now, the only program one needs to run next to the tests is the PubSub test-server.
Note: The tests will still fail when using Qt 6 (which is still marked as experimental and thus not tested). I'll make another PR to fix that and add tests there.