Skip to content

fix: use serverReceivedTime over parseTime for stacking - #5824

Merged
pajlada merged 6 commits into
Chatterino:masterfrom
Nerixyz:fix/test
Jan 19, 2025
Merged

pajlada merged 6 commits into
Chatterino:masterfrom
Nerixyz:fix/test

Conversation

@Nerixyz

@Nerixyz Nerixyz commented Jan 14, 2025

Copy link
Copy Markdown
Contributor

The tests are dying. Twitch hates us. The animals are leaving. The aliens aren’t contacting us. We might be alone. It just might be you and me. But that’s okay. Because do you really need anyone else?

Anyways, this replaces the use of parseTime with serverReceivedTime, because parseTime isn't set for regular messages (it's set to the current time, meaning our tests will be correct just once throughout the day).

I marked this as a draft, because I don't have time and this needs tests for the stacking of regular timeout messages. If you want to add some, feel free to push to this branch.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread src/common/Channel.hpp
Comment thread src/common/Channel.hpp
Comment thread src/messages/MessageSink.hpp
Comment thread src/messages/MessageSink.hpp
Comment thread src/util/VectorMessageSink.cpp
Comment thread src/util/VectorMessageSink.cpp
Comment thread src/util/VectorMessageSink.hpp
Comment thread src/util/VectorMessageSink.hpp

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Comment thread src/util/ChannelHelpers.hpp
Comment on lines +82 to +83
"messageText": "twitchdev has been timed out for 10m. (3 times) ",
"searchText": "twitchdev has been timed out for 10m. (3 times) ",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong, but that's the current behavior.

@Nerixyz
Nerixyz marked this pull request as ready for review January 15, 2025 20:45
@Nerixyz Nerixyz mentioned this pull request Jan 15, 2025
4 tasks done
@pajlada
pajlada enabled auto-merge (squash) January 19, 2025 11:46
@pajlada
pajlada merged commit c195925 into Chatterino:master Jan 19, 2025
babicekmarek pushed a commit to babicekmarek/streamally-chatterino that referenced this pull request Sep 14, 2026
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.

2 participants