✅ Remove special casing for tests from amp-mustache extension registr…

…ation (ampproject#22598) * Remove special casing for tests from amp-mustache production code * Remove all `unregisterTemplate`s
JamesonPrice14 · Jun 4, 2019 · d5acade · d5acade
1 parent 7e8509a
commit d5acade
Show file tree

Hide file tree

Showing 5 changed files with 1 addition and 54 deletions.
diff --git a/...-transform-amp-extension-call/test/fixtures/transform-call/transform-simple-body/input.js b/...-transform-amp-extension-call/test/fixtures/transform-call/transform-simple-body/input.js
@@ -15,15 +15,6 @@
  */
 
 AMP.extension('amp-mustache', '0.2', function(AMP) {
-   // First, unregister template to avoid "Duplicate template type"
-  // error due to multiple versions of amp-mustache in the same unit test run.
-  // This is due to transpilation of test code to ES5 which uses require() and,
-  // unlike import, causes side effects (AMP.registerTemplate) to be run.
-  // For unit tests, it doesn't actually matter which version of amp-mustache is
-  // registered. Integration tests should only have one script version included.
-  if (getMode().test) {
-    Services.templatesFor(window).unregisterTemplate(TAG);
-  }
   AMP.registerTemplate(TAG, AmpMustache);
 });
 

diff --git a/...transform-amp-extension-call/test/fixtures/transform-call/transform-simple-body/output.js b/...transform-amp-extension-call/test/fixtures/transform-call/transform-simple-body/output.js
@@ -14,16 +14,6 @@
  * limitations under the License.
  */
 (function (AMP) {
-  // First, unregister template to avoid "Duplicate template type"
-  // error due to multiple versions of amp-mustache in the same unit test run.
-  // This is due to transpilation of test code to ES5 which uses require() and,
-  // unlike import, causes side effects (AMP.registerTemplate) to be run.
-  // For unit tests, it doesn't actually matter which version of amp-mustache is
-  // registered. Integration tests should only have one script version included.
-  if (getMode().test) {
-    Services.templatesFor(window).unregisterTemplate(TAG);
-  }
-
   AMP.registerTemplate(TAG, AmpMustache);
 })(self.AMP);
 

diff --git a/extensions/amp-mustache/0.1/amp-mustache.js b/extensions/amp-mustache/0.1/amp-mustache.js
@@ -14,9 +14,7 @@
  * limitations under the License.
  */
 
-import {Services} from '../../../src/services';
 import {dict} from '../../../src/utils/object';
-import {getMode} from '../../../src/mode';
 import {isExperimentOn} from '../../../src/experiments';
 import {iterateCursor, templateContentClone} from '../../../src/dom';
 import {
@@ -155,14 +153,5 @@ export class AmpMustache extends AMP.BaseTemplate {
 }
 
 AMP.extension(TAG, '0.1', function(AMP) {
-  // First, unregister template to avoid "Duplicate template type"
-  // error due to multiple versions of amp-mustache in the same unit test run.
-  // This is due to transpilation of test code to ES5 which uses require() and,
-  // unlike import, causes side effects (AMP.registerTemplate) to be run.
-  // For unit tests, it doesn't actually matter which version of amp-mustache is
-  // registered. Integration tests should only have one script version included.
-  if (getMode().test) {
-    Services.templatesFor(window).unregisterTemplate(TAG);
-  }
   AMP.registerTemplate(TAG, AmpMustache);
 });
diff --git a/extensions/amp-mustache/0.2/amp-mustache.js b/extensions/amp-mustache/0.2/amp-mustache.js
@@ -14,9 +14,7 @@
  * limitations under the License.
  */
 
-import {Services} from '../../../src/services';
 import {dict} from '../../../src/utils/object';
-import {getMode} from '../../../src/mode';
 import {isExperimentOn} from '../../../src/experiments';
 import {iterateCursor, templateContentClone} from '../../../src/dom';
 import {purifyHtml, purifyTagsForTripleMustache} from '../../../src/purifier';
@@ -146,14 +144,5 @@ export class AmpMustache extends AMP.BaseTemplate {
 }
 
 AMP.extension(TAG, '0.2', function(AMP) {
-  // First, unregister template to avoid "Duplicate template type"
-  // error due to multiple versions of amp-mustache in the same unit test run.
-  // This is due to transpilation of test code to ES5 which uses require() and,
-  // unlike import, causes side effects (AMP.registerTemplate) to be run.
-  // For unit tests, it doesn't actually matter which version of amp-mustache is
-  // registered. Integration tests should only have one script version included.
-  if (getMode().test) {
-    Services.templatesFor(window).unregisterTemplate(TAG);
-  }
   AMP.registerTemplate(TAG, AmpMustache);
 });
diff --git a/src/service/template-impl.js b/src/service/template-impl.js
@@ -16,8 +16,7 @@
 
 import {Deferred} from '../utils/promise';
 import {Services} from '../services';
-import {dev, devAssert, userAssert} from '../log';
-import {getMode} from '../mode';
+import {dev, userAssert} from '../log';
 import {getService, registerServiceBuilder} from '../service';
 import {rootNodeFor, scopedQuerySelector} from '../dom';
 
@@ -374,17 +373,6 @@ export class Templates {
     }
   }
 
-  /**
-   * For testing only.
-   * @param {string} type
-   * @visibleForTesting
-   */
-  unregisterTemplate(type) {
-    devAssert(getMode().test, 'Should only be used in test mode.');
-    delete this.templateClassMap_[type];
-    delete this.templateClassResolvers_[type];
-  }
-
   /**
    * @param {!BaseTemplate} impl
    * @param {!JsonObject} data