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

test: add snapshot tests for MessageBuilder #5598

Merged
merged 35 commits into from
Oct 13, 2024

Conversation

Nerixyz
Copy link
Contributor

@Nerixyz Nerixyz commented Sep 16, 2024

I want to refactor the MessageBuilder for IRC messages, but it's an integral part of Chatterino, so I'm adding some tests first. These test that the output message is the same. Adding tests requires the following steps (also documented in the MessageBuilder test (at the top).

  1. Add an IRC message in tests/snapshots/MessageBuilder/IRC as a new JSON file ({ "input": "<IRC-msg>" }) and save it
  2. Set UPDATE_SNAPSHOTS in tests/src/MessageBuilder.cpp to true
  3. In the same file, add the filename from step 1 without the extension to IRC_SNAPSHOTS
  4. Run the new test (or run all message-builder tests with ctest -R TestMessageBuilderP --output-on-failure). Don't worry about the integrity test for now.
  5. Set UPDATE_SNAPSHOTS back to false
  6. Ensure the output is stable by running the message-builder tests again ctest -R TestMessageBuilderP --output-on-failure (everything should pass now).

On Windows, you might see that all snapshot files will show as modified. However, after staging them (git add), they shouldn't show up as modified anymore (as just the line-endings changed).

Coverage from this PR: https://app.codecov.io/github/Chatterino/chatterino2/pull/5598

Copy link
Contributor

@github-actions github-actions bot left a comment

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

src/messages/MessageElement.hpp Outdated Show resolved Hide resolved
@Nerixyz Nerixyz changed the title test: add fixture tests for MessageBuilder test: add snapshot tests for MessageBuilder Oct 5, 2024
@Nerixyz Nerixyz marked this pull request as ready for review October 5, 2024 13:52
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

Partial review, haven't taken a full look
Overall looks good and will definitely bring confidence in changing the MessageBuilder logic going forward

tests/src/MessageBuilder.cpp Outdated Show resolved Hide resolved
tests/src/MessageBuilder.cpp Outdated Show resolved Hide resolved
src/messages/MessageBuilder.cpp Outdated Show resolved Hide resolved
src/messages/MessageBuilder.cpp Outdated Show resolved Hide resolved
src/messages/MessageBuilder.cpp Outdated Show resolved Hide resolved
src/providers/ffz/FfzBadges.cpp Show resolved Hide resolved
src/providers/ffz/FfzBadges.cpp Show resolved Hide resolved
src/providers/twitch/TwitchAccount.cpp Show resolved Hide resolved
src/providers/twitch/TwitchChannel.cpp Show resolved Hide resolved
tests/src/MessageBuilder.cpp Outdated Show resolved Hide resolved
@pajlada pajlada enabled auto-merge (squash) October 13, 2024 10:08
@pajlada pajlada merged commit d945331 into Chatterino:master Oct 13, 2024
18 checks passed
@Nerixyz Nerixyz deleted the test/message-builder branch October 13, 2024 10:39
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