8000 improving testing by oozoofrog · Pull Request #215 · ioskrew/SwiftLayout · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

improving testing #215

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Apr 5, 2022
Merged

improving testing #215

merged 10 commits into from
Apr 5, 2022

Conversation

oozoofrog
Copy link
Collaborator
@oozoofrog oozoofrog commented Apr 4, 2022 •
edited
Loading

#213

implementation test cases now has a solid, detailed and concrete structure by @gmlwhdtjd

so, we can now make more flexible api for DSL testing with layout API.

  • flexible and non-invasive testing constraint
  • testing view
  • flexible testing view (how possible)(someday)

@oozoofrog oozoofrog self-assigned this Apr 4, 2022
@oozoofrog oozoofrog added enhancement New feature or request test in progressing Working on labels Apr 4, 2022
@oozoofrog oozoofrog linked an issue Apr 4, 2022 that may be closed by this pull request
improvement for test cases #213
Closed
2 tasks
gmlwhdtjd
gmlwhdtjd previously approved these changes Apr 5, 2022
View reviewed changes
Copy link
Contributor
@gmlwhdtjd gmlwhdtjd left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@oozoofrog oozoofrog requested a review from gmlwhdtjd April 5, 2022 04:48
@oozoofrog
Copy link
Collaborator Author
oozoofrog commented Apr 5, 2022 •
edited
Loading

@gmlwhdtjd apply new assert function to AnchorsImplementsTests

and seems like below if failure

1 subview.leading == superview.leading -----------X-- subview.trailing == siblingview.trailing - 13
2 subview.trailing == superview.trailing ---------X---- subview.leading == siblingview.leading + 13
3 subview.leading == superview.leading + 13 ------X------- subview.trailing == siblingview.trailing
4 subview.trailing == superview.trailing - 13 ----X--------- subview.leading == siblingview.leading
5 subview.leading == siblingview.leading ---------X---- subview.trailing == superview.trailing - 13
6 subview.trailing == siblingview.trailing -------X------ subview.leading == superview.leading + 13
7 subview.leading == siblingview.leading + 13 ----X--------- subview.trailing == superview.trailing
8 subview.trailing == siblingview.trailing - 13 --X----------- subview.leading == superview.leading

1 subview.leading == superview.leading -----------O----------- subview.leading == superview.leading
2 subview.trailing == superview.trailing ---------O--------- subview.trailing == superview.trailing
3 subview.leading == superview.leading + 13 ------O------ subview.leading == superview.leading + 13
4 subview.trailing == superview.trailing - 13 ----O---- subview.trailing == superview.trailing - 13
5 subview.leading == siblingview.leading ---------O--------- subview.leading == siblingview.leading
6 subview.trailing == siblingview.trailing -------O------- subview.trailing == siblingview.trailing
7 subview.leading == siblingview.leading + 13 ----X-- subview.trailing == siblingview.trailing - 13
8 subview.trailing == siblingview.trailing - 13 --X---- subview.leading == siblingview.leading + 13

i think result message will can making better. may be through another pr.

@oozoofrog oozoofrog removed the in progressing Working on label Apr 5, 2022
gmlwhdtjd
gmlwhdtjd reviewed Apr 5, 2022
View reviewed changes
Copy link
Contributor
@gmlwhdtjd gmlwhdtjd left a comment

Choose a reason for hiding this comment

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

In my opinion, it is better to check the difference by directly comparing the values ​​rather than comparing the descriptions.

@oozoofrog
Copy link
Collaborator Author
oozoofrog commented Apr 5, 2022 •
edited
Loading

In my opinion, it is better to check the difference by directly comparing the values ​​rather than comparing the descriptions.

i think comparing descriptions have same result with directly compare, and can make easily readable error. descriptions have rich information for values already. are you worried about anything?

@gmlwhdtjd
Copy link
Contributor

i think comparing descriptions have same result with directly compare, and can make easily readable error. are you worried about anything?

I'm worried that if the descroption is not what I expected, there might be a problem with the test.
Also, as XCTViewAssert.swift grows, I think I need to test XCTViewAssert too.
If the XCTViewAssert is written incorrectly, there is no guarantee that the test will achieve sufficient validation.

@oozoofrog
Copy link
Collaborator Author
oozoofrog commented Apr 5, 2022 •
edited
Loading

i think comparing descriptions have same result with directly compare, and can make easily readable error. are you worried about anything?

I'm worried that if the descroption is not what I expected, there might be a problem with the test. Also, as XCTViewAssert.swift grows, I think I need to test XCTViewAssert too. If the XCTViewAssert is written incorrectly, there is no guarantee that the test will achieve sufficient validation.

faith of solid test case is from passing through red phase, green phases can be corrupt anytime. actually we cannot believe XCTest API, and logic from us or somebody, because its user is human and source creator human too(may be you worried about like this). so this new test function passed through red phase. its should be ok to additional test cases.

and descriptions, my opinion we should believe its consistency. sometimes, we are also dependent to it when debugging too. in fact, we can't even believe equal function like native API, in some cases. in the end, the red phase that passed gives an answer, and this confident that the cases of this PR will be no problem.

so how about this? commit test cases for red phases and to green phase commit for after. @gmlwhdtjd.
with TDD i should commit for red phase. but I didn't. my fault.

@gmlwhdtjd
Copy link
Contributor

@oozoofrog
I agree with your opinion. It seems that my worries were too deep worries.

The development method of committing the red phase and turning it into a green phase looks good.
It is not a rule that must be followed, but how about set it to a recommended level?

gmlwhdtjd
gmlwhdtjd approved these changes Apr 5, 2022
View reviewed changes
Copy link
Contributor
@gmlwhdtjd gmlwhdtjd left a comment

Choose a reason for hiding this comment

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

👍

@oozoofrog
Copy link
Collaborator Author

very agreed

@oozoofrog oozoofrog merged commit 8a5aa5d into develop Apr 5, 2022
@oozoofrog oozoofrog deleted the feature/easy-reading-for-testing branch April 5, 2022 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmlwhdtjd gmlwhdtjd gmlwhdtjd approved these changes

Assignees

@oozoofrog oozoofrog

Labels
enhancement New feature or request test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

improvement for test cases
2 participants
@oozoofrog @gmlwhdtjd
0