Skip to content
New issue

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

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

Already on GitHub? # to your account

test: Test case for extended SpriteButtonComponent #3318

Merged
merged 12 commits into from
Oct 4, 2024

Conversation

puneet-739
Copy link
Contributor

Description

Added an extra test case for testing SpriteButtonComponent class.
which includes a CustomSpriteButtonComponet class to test the working with onLoad instead of directly using Constructor.

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Related Issues

It may help working on the issues like #3293

@puneet-739 puneet-739 changed the title test: Added Test case to test CustomSpriteButtonComponent. test: Added Test case to test CustomSpriteButtonComponent Sep 21, 2024
Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Tests for the SpriteButtonComponent should live in the sprite_button_component_test.dart file.

@puneet-739
Copy link
Contributor Author

Tests for the SpriteButtonComponent should live in the sprite_button_component_test.dart file.

done

Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

These tests are just copies of already existing tests, but going through an extended version that isn't exposed to the user. I don't think we want this type of testing pattern in the code-base.
If you disagree with a good argument I can open it again, but I'll close it for now.

@spydon spydon closed this Oct 3, 2024
@puneet-739
Copy link
Contributor Author

These tests are just copies of already existing tests, but going through an extended version that isn't exposed to the user. I don't think we want this type of testing pattern in the code-base. If you disagree with a good argument I can open it again, but I'll close it for now.

Oh, ok.
Actually, I've added the test cases due to the issue #3293. (I think there is still an issue) Please refer to this comment #3302 (comment)
The reason of adding the test cases is to test the working of SpriteButtonComponent with extended version. because the issue #3293 only occurs when the user use the extended version but not on the Constructor.
So, we need to test if the issue is resolved for the extended version as well, as the present test cases only test for Constructor version, it will always give positive result (it gives error if we use the extended version).

@spydon
Copy link
Member

spydon commented Oct 3, 2024

@puneet-739 you're right, can you try #3327? It should solve the issue so that the user doesn't have to call super.onLoad last.
EDIT: Needs another fix, working on it now.

@spydon spydon reopened this Oct 3, 2024
@spydon
Copy link
Member

spydon commented Oct 3, 2024

@puneet-739 I changed my mind, due to the intricacies of the SpriteButtonComponent it's probably good to have these tests so that we don't get any regressions later. Can you fix the DCM warning? (Make the CustomSpriteButtonComponent private)

@spydon spydon changed the title test: Added Test case to test CustomSpriteButtonComponent test: Test case for extended SpriteButtonComponent Oct 3, 2024
@spydon spydon enabled auto-merge (squash) October 4, 2024 14:27
@spydon spydon merged commit e179cc7 into flame-engine:main Oct 4, 2024
7 of 8 checks passed
@puneet-739 puneet-739 deleted the test-case-for-CustomSpriteButton branch October 4, 2024 14:37
luanpotter pushed a commit that referenced this pull request Oct 15, 2024
Added an extra test case for testing `SpriteButtonComponent` class. 
which includes a `CustomSpriteButtonComponet` class to test the working
with `onLoad` instead of directly using Constructor.

---------

Co-authored-by: Lukas Klingsbo <lukas.klingsbo@gmail.com>
Co-authored-by: Lukas Klingsbo <me@lukas.fyi>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants