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.

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

Add maestro e2e test for Legacy Style Event #46784

Closed
wants to merge 1 commit into from
Closed

Add maestro e2e test for Legacy Style Event #46784

wants to merge 1 commit into from

Conversation

tarunrajput
Copy link
Contributor

@tarunrajput tarunrajput commented Oct 2, 2024 •
edited
Loading

Summary:

part of #46757
closes ME2E0006, ME2E0007

Changelog:

[Internal] - add e2e test for Legacy Style Event

Test Plan:

yarn e2e-test-ios
yarn e2e-test-android

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 2, 2024
@tarunrajput tarunrajput changed the title [CI][Testing] Add maestro e2e test for Legacy Style Event Add maestro e2e test for Legacy Style Event Oct 2, 2024
@tarunrajput
Copy link
Contributor Author

/test-e2e

@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 2, 2024
cipolleschi
cipolleschi approved these changes Oct 2, 2024
View reviewed changes
Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

Nice job! Let's see if CI is green!

@tarunrajput
Copy link
Contributor Author

@cipolleschi, By mistake, I opened the PR for ME2E0006 and ME2E0007 instead of ME2E0005 and ME2E0006. Could you please update the task assignment?

@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

cipolleschi
cipolleschi requested changes Oct 10, 2024
View reviewed changes
Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. There is a logical issues that we should address before merging this. I left a comment in the code

packages/rn-tester/NativeComponentExample/js/MyNativeView.js Outdated
// $FlowFixMe[incompatible-call]
ref.current,
);
setLegacyStyleEventCount(prevCount => prevCount + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

we should call setLegacyStyleCount when the event is received by JS. It should be line 126. Otherwise we are not actually tested that the event is sent and received back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@cipolleschi cipolleschi mentioned this pull request Oct 10, 2024
Open
19 tasks
@tarunrajput tarunrajput force-pushed the e2e/add-legacy-style-fire-event-test branch from 20440d3 to 6478ea2 Compare October 10, 2024 11:07
@tarunrajput
Copy link
Contributor Author

/test-e2e

cipolleschi
cipolleschi approved these changes Oct 15, 2024
View reviewed changes
Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

@tarunrajput thanks for working on this and for applying the review suggestions!

@facebook-github-bot
Copy link
Contributor

@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Oct 15, 2024
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in 3f63347.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @tarunrajput in 3f63347

When will my fix make it into a release? | How to file a pick request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cipolleschi cipolleschi cipolleschi approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tarunrajput @facebook-github-bot @react-native-bot @cipolleschi