[doctor command] Add tests for androidSDK and androidNDK

[thib92]

Summary:

Add unit tests for the healthchecks for androidSDK and androidNDK

Also removed a $FlowFixMe that wasn't removed when the androidSDK file was moved to TS.

Related to #694

Test Plan:

CI is green

[thib92]

@lucasbento tests are failing on Windows because apparently the androidSDK check specific to Windows is not responding within 5s.

I'm not too sure why but I'm suspecting the execa call here is not responding. Should we put a timeout on it?

cli/packages/cli/src/commands/doctor/healthchecks/androidSDK.ts

Lines 24 to 29 in ed0a6fd

	const {stdout} = await execa(
	process.env.ANDROID_HOME
	? `${process.env.ANDROID_HOME}/tools/bin/sdkmanager`
	: 'sdkmanager',
	['--list'],
	);

[lucasbento]

@thib92: I would say so, it seems like a command that can take quite a while to respond.

I'm not sure if 5 seconds would be sufficient though, @thymikee can you tell us about that?

I didn't actually do this piece of code so I am not sure about it.

[thymikee]

I'd mock a response that we expect, no time for long timeouts.

[lucasbento]

Yes, that's definitely a better approach 😄

[lucasbento]

Just a few comments, good work as always 🙂

[thib92]

@lucasbento thanks for the good review. Everything should be fixed.

[lucasbento]

Awesome, thanks @thib92!

[thib92]

Btw I'm doing a lot of copy-paste between all the healthcheck tests. Once I'm done with all of them, I'll see if I can create util functions to avoid repeating code in the tests.

[lucasbento]

@thymikee: can you please give a review?

[thymikee]

jest.mock is hosited to the top of the parent scope, it's not meant to be called mutliple times. Can you move it to the top-level scope, right after imports, so it's a bit more obvious the execa module is mocked?

[thib92]

Changed it a little bit to be easier to understand and more standard. What do you think?

[thymikee]

Let's rename it to mockSdkManagerResponse or something so that it's closer to what it's actually doing, since we don't extract execa call to a function we could mock – which would make it obvious what is mocked.

[thib92]

See what I changed. What do you think?

[thymikee]

LGTM, small things to adjust