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

ci: fixed detox tests. #1446

Merged
merged 41 commits into from
Feb 22, 2024
Merged

ci: fixed detox tests. #1446

merged 41 commits into from
Feb 22, 2024

Conversation

zhyd1997
Copy link
Contributor

Close: #889

@zhyd1997 zhyd1997 marked this pull request as draft November 26, 2023 09:57
@zhyd1997
Copy link
Contributor Author

Will update again in the weekend.

@zhyd1997 zhyd1997 closed this Dec 1, 2023
@zhyd1997 zhyd1997 reopened this Dec 1, 2023
@zhyd1997
Copy link
Contributor Author

zhyd1997 commented Dec 2, 2023

Still something wrong on e2e:

Screenshot 2023-12-02 at 17 38 51

zhyd1997
zhyd1997 commented Dec 3, 2023
View reviewed changes
test/scripts/generate-react-native-app.sh Outdated
Comment on lines 19 to 21
# pin specific version of expo
npm i [email protected]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's an issue in the latest expo, which mean e2e will fails in the local env as well:

Screenshot 2023-12-03 at 10 44 05

expo/expo#25551

Copy link
Contributor Author

@zhyd1997 zhyd1997 Dec 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

But it failed with downgraded `expo`: Screenshot 2023-12-03 at 19 47 48

Copy link
Contributor Author

@zhyd1997 zhyd1997 Dec 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

same with the latest `expo`: Screenshot 2023-12-03 at 19 51 51

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Recorded video in CI:
test.mp4

zhyd1997
zhyd1997 commented Dec 3, 2023
View reviewed changes
generators/app/templates/e2e/config.json.ejs
@@ -1,8 +1,12 @@
{
"testEnvironment": "./environment",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

considering rewrite reloadApp, or maybe occur an issue: wix/Detox#3992

zhyd1997
zhyd1997 commented Dec 3, 2023
View reviewed changes
generators/app/templates/package.json
@@ -34,7 +34,6 @@
},
"devDependencies": {
"@react-native-community/eslint-config": "3.2.0",
"@ruddell/detox-expo-helpers": "0.7.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed it because it contains outdated @expo/webpack-config and it is not maintained no longer.

@zhyd1997
Copy link
Contributor Author

zhyd1997 commented Dec 5, 2023 •
edited
Loading

Update: Failed on the real machine (Mac OS 14.1.2 (23B92)): Screenshot 2023-12-05 at 23 33 50

Fixed ff330f4

@zhyd1997
Copy link
Contributor Author

Just some interesting findings:

Circle CI works for now (Xcode v15.1.0 (latest) - green):

Screenshot 2023-12-17 at 20 26 25

but e2e CI failed in github actions (Xcode v15.0.1 (latest) - timeout)

@mraible @mshima PTAL

@zhyd1997 zhyd1997 marked this pull request as ready for review December 17, 2023 12:28
@mshima mshima closed this Dec 17, 2023
@mshima
Copy link
Member

mshima commented Dec 20, 2023 •
edited
Loading

It's already an improvement.
But but tests should pass so we can add as requirement to merge otherwise dependabot can break it again.
For now I recommend to skip failing tests.

@zhyd1997 zhyd1997 force-pushed the ci/ticket-889-1700974455106 branch from e58a468 to 5e2d8c1 Compare December 21, 2023 01:31
@zhyd1997 zhyd1997 force-pushed the ci/ticket-889-1700974455106 branch from 5e2d8c1 to 4bd9c03 Compare December 21, 2023 01:40
@mshima
Copy link
Member

mshima commented Dec 21, 2023

You should use describe.skip instead of filter test.

@mshima mshima requested a review from mraible December 21, 2023 16:53
@zhyd1997
Copy link
Contributor Author

zhyd1997 commented Dec 23, 2023 •
edited
Loading

Hi @mshima @mraible

I have upgraded Xcode from 14 to 15.1 (latest version currently, which is added 3 days ago, but it's unstable sometimes) because:

  1. all of tests are passed on my machine using XCode 15.1.0 and IOS simulator iPhone 15+.
  2. we always maintain dependencies but not for e2e test enviroment, so I handled it in this pull request.

Please let me know if you have any questions!

Merry Chrismas! 🎄🧑‍🎄

@zhyd1997
Copy link
Contributor Author

zhyd1997 commented Dec 23, 2023 •
edited
Loading

and seems that Circle CI is more reliable (always passed just like real machine) and faster than Github Actions (sometimes).

Screenshot 2023-12-23 at 20 42 39

@mraible
Copy link
Contributor

mraible commented Feb 21, 2024

@zhyd1997 Can you please fix the conflicts in this PR?

@mshima
Copy link
Member

mshima commented Feb 22, 2024

I thought I had merged this PR =\

@mshima mshima merged commit ff342b5 into jhipster:main Feb 22, 2024
14 checks passed
@DanielFran DanielFran mentioned this pull request Feb 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mraible mraible Awaiting requested review from mraible

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix Detox tests running in GitHub Actions
3 participants
@zhyd1997 @mshima @mraible