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 future gmsaas incompatibility #4361

Closed
1 of 2 tasks
d4vidi opened this issue Feb 7, 2024 · 9 comments
Closed
1 of 2 tasks

Fix future gmsaas incompatibility #4361

d4vidi opened this issue Feb 7, 2024 · 9 comments

Comments

@d4vidi
Copy link
Collaborator

d4vidi commented Feb 7, 2024

What happened?

According to the gmsaas 1.10.0 documentation:

Marked gmsaas auth whoami command as deprecated.

Since it is used regularly by Detox, it will break in future versions.

cc @igorgn @lilinor

What was the expected behaviour?

N/A

Was it tested on latest Detox?

  • I have tested this issue on the latest Detox release and it still reproduces.

Did your test throw out a timeout?

Help us reproduce this issue!

No response

In what environment did this happen?

Detox version:
React Native version:
Has Fabric (React Native's new rendering system) enabled: (yes/no)
Node version:
Device model:
Android version:
Test-runner (select one): jest / other

Detox logs

N/A

Device logs

N/A

More data, please!

No response

@lilinor
Copy link
Contributor

lilinor commented Feb 7, 2024

Thanks for mentioning @d4vidi ! We released the authentication with API tokens and it introduced the deprecation of this command. For which reason do you need to use this command in the integration?

@noomorph
Copy link
Collaborator

noomorph commented Feb 7, 2024

@lilinor we have this logic:

async function _validateGmsaasAuth() {
  if (!await this._authService.getLoginEmail()) {
    throw new DetoxRuntimeError({
      message: `Cannot run tests using 'android.genycloud' type devices, because Genymotion was not logged-in to!`,
      hint: `Log-in to Genymotion-cloud by running this command (and following instructions):\n${environment.getGmsaasPath()} auth login --help`,
    });
  }
}

Perhaps we could also check for GENYMOTION_API_TOKEN as well, but would that be enough? I'm not sure I understand what gmsaas auth token ... does.

@d4vidi
Copy link
Collaborator Author

d4vidi commented Feb 7, 2024

Yep that's the code. Thanks @noomorph.
@lilinor we pre-validate that gmsaas is properly installed and set up, that's all. Would also appreciate an advice regarding what we might be able to do equivalently.

@lilinor
Copy link
Contributor

lilinor commented Feb 8, 2024

gmsaas auth whoami is deprecated but rest assured that it won’t be removed unless we have your approval so it won’t break current gmsaas installations with username/pwd authentication. But we have just discovered that detox tests will break if users authenticate with api token as we return the error message : Error: command not supported when using API Token We can do a quick fix where gmsaas auth whomai will return exit code 0 if an API token is set.

We plan in the future versions of gmsaas to add a gmsaas doctor command to verify the whole gmsaas environment which will work with credentials & API token authentication :

  • Android SDK

  • Authentication with a real API call as gmsaas auth whoami only returns the email stored in the config file, there isn’t real check, password could have changed and running the other gmsaas commands like gmsaas list instances, start would fail

Does this sound ok for you?

@noomorph
Copy link
Collaborator

noomorph commented Feb 8, 2024

@lilinor IMO, a new command like gmsaas auth status may also work.

The idea with exit code = 0/1 sounds good to me.

As for hotfixing the gmsaas auth whoami command, I think it won't be semantic enough for tokens, as it cannot respond conceptually correctly to the question "Who am I?" just because tokens don't work the same.

The idea with gmsaas doctor sounds good as well.

@lilinor
Copy link
Contributor

lilinor commented Feb 9, 2024

As you said, using gmsaas auth whoami for API token doesn’t make any sense conceptually. As all our detox users haven’t switched to API tokens, we could also leave it like this and tell them to stick to username/password waiting for gmsaas doctor to come out if they encounter the issue. We could aim to release this command within the month.

Copy link

stale bot commented Mar 17, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you believe the issue is still relevant, please test on the latest Detox and report back.

Thank you for your contributions!

For more information on bots in this repository, read this discussion.

@stale stale bot added the 🏚 stale label Mar 17, 2024
@d4vidi d4vidi closed this as completed Mar 20, 2024
@lilinor
Copy link
Contributor

lilinor commented Apr 4, 2024

Hi @d4vidi & @noomorph !
Just letting you know that we released gmsaas doctor command in our last 1.11.0 version of gmsaas :)

@d4vidi
Copy link
Collaborator Author

d4vidi commented Apr 4, 2024

Hi @d4vidi & @noomorph ! Just letting you know that we released gmsaas doctor command in our last 1.11.0 version of gmsaas :)

Hey Thanks! We have a backlogged issue for that: #4417
Hopefully, we'll get around to it soon enough.

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

No branches or pull requests

3 participants