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.

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

fix:-fixes system imagewith the updated android image #23

Merged
merged 7 commits into from
Mar 6, 2024

Conversation

Biki-das
Copy link
Contributor

@Biki-das Biki-das commented Feb 24, 2024

test suits while running yarn test:unit had 3 failing tests which was due to the test assertion being compared with old system image of android, this updates the same which is being used and now tests are passing, this changes the image to arm64-v8a

Current Behaviour
https://github.com/nightwatchjs/mobile-helper-tool/assets/72331432/d5b8c4c4-8948-4f9f-b770-a1c19439f7df

Updated Behaviour after fixes
https://github.com/nightwatchjs/mobile-helper-tool/assets/72331432/8e9fad42-8e65-4471-8c30-83529f531426

Screenshot 2024-02-24 at 1 39 35 PM

@Biki-das
Copy link
Contributor Author

@garg3133, i see there is no prettier config in this project? is it purposely done, since while i was fixing this, prettier was formatting the lines and had to disable prettier, would you like to have a prettier configuration in this Project? happy to setup the same.

@garg3133
Copy link
Member

@Biki-das The tests are failing but I'll take a closer look at the PR in a while. As for the Prettier command, we already ESLint setup for the project and don't need to set up Prettier as well.

@garg3133
Copy link
Member

@Biki-das The tests you pointed out only fail on macOS systems with M-series chipsets (which have ARM-based architecture), so a better solution for this would be to check if the system on which the tests are running on are ARM-based macOS systems or not, assert different things in both the cases.

There's already some logic written on finding out if the system in ARM-based or not in the codebase, you can go and look for it.

@Biki-das
Copy link
Contributor Author

Biki-das commented Feb 24, 2024

@garg3133 Thanks for the review, yes you are correct, it's failing for arm64 bases arch which is limited to apple silicon ,
and also found out the ABI util function which determines if system is arm based or not, used the same now to conditionally test the same.

Co-authored-by: Priyansh Garg <priyanshgarg30@gmail.com>
@garg3133
Copy link
Member

That works! Can you make the changes at the other places as well?

@garg3133 garg3133 merged commit 5c4931a into nightwatchjs:main Mar 6, 2024
2 checks passed
@Biki-das
Copy link
Contributor Author

Biki-das commented Mar 6, 2024

@garg3133 sorry for the radio silence, and we gotta merge!
thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants