Repository: wix/Detox (js)

Issue:
Fail Detox on genymotion alloc disambig
### Describe your idea

This is an undesired deduping mechanism, need to remove it:

```
12:34:56.789 detox[1234] i More than one Genymotion-Cloud recipe found for recipe name MyEmulator_SDK34:
--
  | MyEmulator_SDK34_arm64 (<uuid1>)
  | MyEmulator_SDK34 (<uuid2>)
  | Falling back to MyEmulator_SDK34_arm64
```

(you find yourself setting recipe A in the config and end up with recipe B)...


Pull request:
Fix handling of ambiguous genymotion recipe matching
## Description

<!--
Thank you for contributing!

Step 1: Before submitting a pull request that introduces a new functionality or fixes a bug,
please open an issue where we could discuss the suggestion, problem - and potential ways to fix.
-->

- Fixes #4784

<!--
Step 2: Provide an overview of how your fix / enhancement works.
If possible, provide screenshots of the before and after states (even for simple command line options - show the terminal).
-->

In this pull request, I have addressed the issue of Genymotion recipe disambiguation. Added the following:
- Throw an error when more than one recipe matches a query
- Aligned flow of 0 recipe matches: Throw an error instead of returning null. This is a better approach because the recipes service knows the error details best and therefore should be the one raising the problem.

<!--
Step 3: Please review the checklist below.
-->

---


> _For features/enhancements:_
 - [ ] I have added/updated the relevant references in the [documentation](https://github.com/wix/Detox/tree/master/docs) files.

> _For API changes:_
 - [ ] I have made the necessary changes in the [types index](https://github.com/wix/Detox/blob/master/detox/index.d.ts) file.


Patch:
diff --git a/detox/src/devices/allocation/drivers/android/genycloud/GenyAllocDriver.js b/detox/src/devices/allocation/drivers/android/genycloud/GenyAllocDriver.js
index 1d6a68eef..0b0aed18d 100644
--- a/detox/src/devices/allocation/drivers/android/genycloud/GenyAllocDriver.js
+++ b/detox/src/devices/allocation/drivers/android/genycloud/GenyAllocDriver.js
@@ -3,7 +3,6 @@
  * @typedef {import('../../../../common/drivers/android/cookies').GenycloudEmulatorCookie} GenycloudEmulatorCookie
  */
 
-const { DetoxRuntimeError } = require('../../../../../errors');
 const Timer = require('../../../../../utils/Timer');
 const log = require('../../../../../utils/logger').child({ cat: 'device' });
 
@@ -56,7 +55,6 @@ class GenyAllocDriver {
   async allocate(deviceConfig) {
     const deviceQuery = deviceConfig.device;
     const recipe = await this._recipeQuerying.getRecipeFromQuery(deviceQuery);
-    this._assertRecipe(deviceQuery, recipe);
 
     let instance = this._genyRegistry.findFreeInstance(recipe);
     if (!instance) {
@@ -151,15 +149,6 @@ class GenyAllocDriver {
     this._reportGlobalCleanupSummary(instances);
   }
 
-  _assertRecipe(deviceQuery, recipe) {
-    if (!recipe) {
-      throw new DetoxRuntimeError({
-        message: `No Genymotion-Cloud template found to match the configured lookup query: ${JSON.stringify(deviceQuery)}`,
-        hint: `Revisit your detox configuration. Genymotion templates list is available at: https://cloud.geny.io/recipes#custom`,
-      });
-    }
-  }
-
   _reportGlobalCleanupSummary(deletionLeaks) {
     if (deletionLeaks.length) {
       log.warn(events.GENYCLOUD_TEARDOWN, 'WARNING! Detected a Genymotion SaaS instance leakage, for the following instances:');
diff --git a/detox/src/devices/allocation/drivers/android/genycloud/GenyRecipeQuerying.js b/detox/src/devices/allocation/drivers/android/genycloud/GenyRecipeQuerying.js
index 6ecd1582b..32b68977e 100644
--- a/detox/src/devices/allocation/drivers/android/genycloud/GenyRecipeQuerying.js
+++ b/detox/src/devices/allocation/drivers/android/genycloud/GenyRecipeQuerying.js
@@ -1,4 +1,7 @@
 class GenyRecipeQuerying {
+  /**
+   * @param {import('./services/GenyRecipesService')} recipesService
+   */
   constructor(recipesService) {
     this.recipesService = recipesService;
   }
diff --git a/detox/src/devices/allocation/drivers/android/genycloud/services/GenyRecipesService.js b/detox/src/devices/allocation/drivers/android/genycloud/services/GenyRecipesService.js
index 3cb221f87..8423003eb 100644
--- a/detox/src/devices/allocation/drivers/android/genycloud/services/GenyRecipesService.js
+++ b/detox/src/devices/allocation/drivers/android/genycloud/services/GenyRecipesService.js
@@ -1,4 +1,4 @@
-const logger = require('../../../../../../utils/logger').child({ cat: 'device' });
+const { DetoxRuntimeError } = require('../../../../../../errors');
 
 const Recipe = require('./dto/GenyRecipe');
 
@@ -10,15 +10,18 @@ class GenyRecipesService {
   async getRecipeByName(recipeName) {
     const { recipes } = await this.genyCloudExec.getRecipe(recipeName);
     if (!recipes.length) {
-      return null;
+      throw new DetoxRuntimeError({
+        message: `No Genymotion-Cloud recipe found for recipe name "${recipeName}"`,
+        hint: `Please check your recipe name or use recipe UUID instead.`,
+      });
     }
 
     if (recipes.length > 1) {
       const recipesInfoList = recipes.map((recipe) => `  ${recipe.name} (${recipe.uuid})`).join('\n');
-      logger.warn(
-        { event: 'GENYCLOUD_RECIPE_LOOKUP' },
-        `More than one Genymotion-Cloud recipe found for recipe name ${recipeName}:\n${recipesInfoList}\nFalling back to ${recipes[0].name}`
-      );
+      throw new DetoxRuntimeError({
+        message: `More than one Genymotion-Cloud recipe found for recipe name ${recipeName}:\n${recipesInfoList}`,
+        hint: `Please specify a unique recipe name or use recipe UUID instead.`,
+      });
     }
     return new Recipe(recipes[0]);
   }


Test patch:
diff --git a/detox/src/devices/allocation/drivers/android/genycloud/services/GenyRecipesService.test.js b/detox/src/devices/allocation/drivers/android/genycloud/services/GenyRecipesService.test.js
index 3238d5aed..c46a6d3b6 100644
--- a/detox/src/devices/allocation/drivers/android/genycloud/services/GenyRecipesService.test.js
+++ b/detox/src/devices/allocation/drivers/android/genycloud/services/GenyRecipesService.test.js
@@ -21,13 +21,9 @@ describe('Genymotion-Cloud recipes service', () => {
     name: 'another-mock-recipe-name',
   });
 
-  let logger;
   let exec;
   let uut;
   beforeEach(() => {
-    jest.mock('../../../../../../utils/logger');
-    logger = require('../../../../../../utils/logger');
-
     const GenyCloudExec = jest.genMockFromModule('../exec/GenyCloudExec');
     exec = new GenyCloudExec();
 
@@ -36,15 +32,14 @@ describe('Genymotion-Cloud recipes service', () => {
   });
 
   describe('getting a recipe by name', () => {
-    it('should return null if no recipes found', async () => {
+    it('should throw an error if no recipes found', async () => {
       givenNoRecipes();
-      expect(await uut.getRecipeByName('mock-name')).toEqual(null);
+      await expect(uut.getRecipeByName('mock-name')).rejects.toThrowErrorMatchingSnapshot();
     });
 
-    it('should return 1st match if there are some recipes', async () => {
+    it('should return the recipe if exactly one match is found', async () => {
       const recipe = aRecipe();
-      const recipe2 = anotherRecipe();
-      givenRecipes(recipe, recipe2);
+      givenRecipes(recipe);
 
       const result = await uut.getRecipeByName(recipe.name);
 
@@ -59,31 +54,12 @@ describe('Genymotion-Cloud recipes service', () => {
       expect(result.constructor.name).toContain('Recipe');
     });
 
-    it('should warn if there are multiple matches', async () => {
+    it('should throw an error if there are multiple matches', async () => {
       const recipe = aRecipe();
       const recipe2 = anotherRecipe();
       givenRecipes(recipe, recipe2);
 
-      await uut.getRecipeByName(recipe.name);
-
-      expect(logger.warn).toHaveBeenCalledWith(
-        { event: 'GENYCLOUD_RECIPE_LOOKUP' },
-        [
-          `More than one Genymotion-Cloud recipe found for recipe name ${recipe.name}:`,
-          `  ${recipe.name} (${recipe.uuid})`,
-          `  ${recipe2.name} (${recipe2.uuid})`,
-          `Falling back to ${recipe.name}`,
-        ].join('\n'),
-      );
-    });
-
-    it('should not warn if there is only 1 match', async () => {
-      const recipe = aRecipe();
-      givenRecipes(recipe);
-
-      await uut.getRecipeByName(recipe.name);
-
-      expect(logger.warn).not.toHaveBeenCalled();
+      await expect(uut.getRecipeByName(recipe.name)).rejects.toThrowErrorMatchingSnapshot();
     });
   });
 
diff --git a/detox/src/devices/allocation/drivers/android/genycloud/services/__snapshots__/GenyRecipesService.test.js.snap b/detox/src/devices/allocation/drivers/android/genycloud/services/__snapshots__/GenyRecipesService.test.js.snap
new file mode 100644
index 000000000..f1a67ccb8
--- /dev/null
+++ b/detox/src/devices/allocation/drivers/android/genycloud/services/__snapshots__/GenyRecipesService.test.js.snap
@@ -0,0 +1,15 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Genymotion-Cloud recipes service getting a recipe by name should throw an error if no recipes found 1`] = `
+"No Genymotion-Cloud recipe found for recipe name "mock-name"
+
+HINT: Please check your recipe name or use recipe UUID instead."
+`;
+
+exports[`Genymotion-Cloud recipes service getting a recipe by name should throw an error if there are multiple matches 1`] = `
+"More than one Genymotion-Cloud recipe found for recipe name mock-recipe-name:
+  mock-recipe-name (mock-recipe-uuid)
+  another-mock-recipe-name (another-mock-recipe-uuid)
+
+HINT: Please specify a unique recipe name or use recipe UUID instead."
+`;