Skip to content

test(item-sliding): re-enable flaky tests - #28192

Merged
thetaPC merged 10 commits into
mainfrom
FW-5021
Sep 20, 2023
Merged

test(item-sliding): re-enable flaky tests#28192
thetaPC merged 10 commits into
mainfrom
FW-5021

Conversation

@thetaPC

@thetaPCthetaPC commented Sep 18, 2023

Copy link
Copy Markdown
Contributor

Issue number: internal

What is the current behavior?

Some of the tests for item-sliding were being skipped due to flakiness.

What is the new behavior?

  • Updated the tests to use the stable function, dragElementBy to handle gestures, removing the gesture flakiness.
  • Separated the basic test to lessen the gesture complexity else it becomes flaky since it can't handle opening and closing and opening in the same test.
  • Tests are now checking all modes and all directions.
  • Updated a utils function with a warning regarding an open issue with RTL.

Does this introduce a breaking change?

  • Yes
  • No

Other information

N/A

@github-actionsgithub-actionsBot added the package: core @ionic/core package label Sep 18, 2023
@thetaPC
thetaPC marked this pull request as ready for review September 18, 2023 21:37
@thetaPC
thetaPC requested review from a team and brandyscarney and removed request for a teamSeptember 18, 2023 21:37
brandyscarney
brandyscarney reviewed Sep 19, 2023
View reviewed changes

@brandyscarneybrandyscarney left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The tests seem to work now, great job! I ran them with --repeat-each=15 and verified there were no failing tests. Just a few comments on the file names but other than that this looks good.

Screenshot 2023-09-19 at 4 12 00 PM

Comment thread...icons/item-sliding.e2e.ts-snapshots/item-sliding-iconsBottom-ios-ltr-Mobile-Chrome-linux.png
Comment thread...st/icons/item-sliding.e2e.ts-snapshots/item-sliding-iconsEnd-ios-ltr-Mobile-Chrome-linux.png
Comment thread...t/icons/item-sliding.e2e.ts-snapshots/item-sliding-iconsOnly-ios-ltr-Mobile-Chrome-linux.png
Comment thread.../icons/item-sliding.e2e.ts-snapshots/item-sliding-iconsStart-ios-ltr-Mobile-Chrome-linux.png
Comment thread...st/icons/item-sliding.e2e.ts-snapshots/item-sliding-iconsTop-ios-ltr-Mobile-Chrome-linux.png
Comment threadcore/src/components/item-sliding/test/test.utils.ts Outdated
Comment threadcore/src/components/item-sliding/test/icons/item-sliding.e2e.ts
thetaPCand others added 2 commits September 19, 2023 14:16
@thetaPC@brandyscarney
test(item-sliding): update todo note
adf9753
Co-authored-by: Brandy Carney <brandyscarney@users.noreply.github.com>
@thetaPC
test(item-sliding): requested changes
b3e5e3d
brandyscarney
brandyscarney reviewed Sep 19, 2023
View reviewed changes
Comment threadcore/src/components/item-sliding/test/icons/item-sliding.e2e.ts Outdated
brandyscarney
brandyscarney reviewed Sep 19, 2023
View reviewed changes
Comment threadcore/src/components/item-sliding/test/icons/item-sliding.e2e.ts Outdated
const itemIDs = ['iconsOnly', 'iconsStart', 'iconsEnd', 'iconsTop', 'iconsBottom'];
for (const itemID of itemIDs) {
await testSlidingItem(page, itemID, itemID, screenshot);
const itemIDKebab = itemID.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();

@brandyscarneybrandyscarneySep 19, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe move this down above the screenshot call and add a comment on why it is here?

thetaPCand others added 2 commits September 19, 2023 16:10
@thetaPC@brandyscarney
test(item-sliding): remove only
aed14c3
Co-authored-by: Brandy Carney <brandyscarney@users.noreply.github.com>
@thetaPC
test(item-sliding): move kebab lower
974298a
brandyscarney
brandyscarney reviewed Sep 20, 2023
View reviewed changes
Comment threadcore/src/components/item-sliding/test/icons/item-sliding.e2e.ts Outdated
thetaPCand others added 2 commits September 20, 2023 08:14
@thetaPC@brandyscarney
test(item-sliding): update comment
52cb7db
Co-authored-by: Brandy Carney <brandyscarney@users.noreply.github.com>
@thetaPC
Merge branch 'main' of github.com:ionic-team/ionic-framework into FW-…
0c7c23c
…5021
@thetaPC
thetaPC added this pull request to the merge queueSep 20, 2023
@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Sep 20, 2023
@thetaPC
thetaPC added this pull request to the merge queueSep 20, 2023
Merged via the queue into main with commit 5e016a6Sep 20, 2023
@thetaPC
thetaPC deleted the FW-5021 branch September 20, 2023 17:54
liamdebeasi pushed a commit that referenced this pull request Sep 22, 2023
@thetaPC@Ionitron
@brandyscarney@liamdebeasi
test(item-sliding): re-enable flaky tests (#28192)
04805f3
Issue number: internal
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
Some of the tests for `item-sliding` were being skipped due to
flakiness.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Updated the tests to use the stable function, `dragElementBy` to
handle gestures, removing the gesture flakiness.
- Separated the basic test to lessen the gesture complexity else it
becomes flaky since it can't handle opening and closing and opening in
the same test.
- Tests are now checking all modes and all directions.
- Updated a utils function with a warning regarding an open issue with
RTL.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
N/A
---------
Co-authored-by: ionitron <hi@ionicframework.com>
Co-authored-by: Brandy Carney <brandyscarney@users.noreply.github.com>
liamdebeasi pushed a commit that referenced this pull request Sep 26, 2023
@thetaPC@Ionitron
@brandyscarney@liamdebeasi
test(item-sliding): re-enable flaky tests (#28192)
6d65787
Issue number: internal
---------
<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->
<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->
## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
Some of the tests for `item-sliding` were being skipped due to
flakiness.
## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->
- Updated the tests to use the stable function, `dragElementBy` to
handle gestures, removing the gesture flakiness.
- Separated the basic test to lessen the gesture complexity else it
becomes flaky since it can't handle opening and closing and opening in
the same test.
- Tests are now checking all modes and all directions.
- Updated a utils function with a warning regarding an open issue with
RTL.
## Does this introduce a breaking change?
- [ ] Yes
- [x] No
<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->
## Other information
<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
N/A
---------
Co-authored-by: ionitron <hi@ionicframework.com>
Co-authored-by: Brandy Carney <brandyscarney@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandyscarneybrandyscarneybrandyscarney approved these changes

Assignees

No one assigned

Labels

package: core@ionic/core package

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@thetaPC@brandyscarney@Ionitron