rewrite tests to avoid using returnValues which for some reason fails

tensorflow · Jul 18, 2022 · d0a7292 · d0a7292
1 parent bc8352b
commit d0a7292
Show file tree

Hide file tree

Showing 6 changed files with 215 additions and 127 deletions.
diff --git a/tensorboard/webapp/app_routing/location.ts b/tensorboard/webapp/app_routing/location.ts
@@ -50,7 +50,7 @@ const utils = {
   },
   getSearch() {
     return window.location.search;
-  }
+  },
 };
 
 @Injectable()

diff --git a/tensorboard/webapp/routes/dashboard_deeplink_provider.ts b/tensorboard/webapp/routes/dashboard_deeplink_provider.ts
@@ -14,7 +14,7 @@ limitations under the License.
 ==============================================================================*/
 import {Injectable} from '@angular/core';
 import {Store} from '@ngrx/store';
-import {combineLatest, of, Observable} from 'rxjs';
+import {combineLatest, Observable, of} from 'rxjs';
 import {map} from 'rxjs/operators';
 import {DeepLinkProvider} from '../app_routing/deep_link_provider';
 import {SerializableQueryParams} from '../app_routing/types';
@@ -97,12 +97,14 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
           return [{key: TAG_FILTER_KEY, value: filterText}];
         })
       ),
-      of(getOverriddenFeatureFlagStates(
-        FeatureFlagMetadataMap as Record<
-          string,
-          FeatureFlagMetadata<FeatureFlagType>
-        >
-      )),
+      of(
+        getOverriddenFeatureFlagStates(
+          FeatureFlagMetadataMap as Record<
+            string,
+            FeatureFlagMetadata<FeatureFlagType>
+          >
+        )
+      ),
       store.select(selectors.getMetricsSettingOverrides).pipe(
         map((settingOverrides) => {
           if (Number.isFinite(settingOverrides.scalarSmoothing)) {

diff --git a/tensorboard/webapp/routes/feature_flag_serializer.ts b/tensorboard/webapp/routes/feature_flag_serializer.ts
@@ -16,31 +16,48 @@ import {Location} from '../app_routing/location';
 import {SerializableQueryParams} from '../app_routing/types';
 import {FeatureFlagMetadata} from '../feature_flag/store/feature_flag_metadata';
 
+/**
+ * Finds all FeatureFlags from the provided FeatureFlagMetadata that are present in the
+ * query params and returns their overridden values.
+ * Note: If a flag has multiple values in the query params it will be returned multiple
+ * times.
+ *
+ * i.e. The query params '?experimentalPlugin=0&experimentalPlugin=1&experimentalPlugin=2'
+ * will result in a return value of
+ * [
+ *   { key: 'experimentalPlugin', value: '0' },
+ *   { key: 'experimentalPlugin', value: '1' },
+ *   { key: 'experimentalPlugin', value: '2' },
+ * ]
+ */
 export function getOverriddenFeatureFlagStates<T>(
-  featureFlagQueryParameters: Record<string, FeatureFlagMetadata<T>>
+  featureFlagMetadataMap: Record<string, FeatureFlagMetadata<T>>
 ): SerializableQueryParams {
-  const currentQueryParams = Object.fromEntries(
-    serializableQueryParamsToEntries(new Location().getSearch() || [])
+  // Converting the array to a map allows for a more efficient filter function below.
+  const currentQueryParams = (new Location().getSearch() || []).reduce(
+    (map, {key, value}) => {
+      if (!map[key]) {
+        map[key] = [];
+      }
+      map[key].push(value);
+
+      return map;
+    },
+    {} as Record<string, string[]>
   );
 
-  const currentlyOverriddenQueryParams = Object.values(
-    featureFlagQueryParameters
-  )
+  const currentlyOverriddenQueryParams = Object.values(featureFlagMetadataMap)
     .map(({queryParamOverride}: FeatureFlagMetadata<T>) => queryParamOverride)
     .filter(
       (queryParamOverride) =>
         queryParamOverride && queryParamOverride in currentQueryParams
     ) as string[];
-  return currentlyOverriddenQueryParams.map((queryParamOverride) => {
-    return {
-      key: queryParamOverride,
-      value: currentQueryParams[queryParamOverride],
-    };
-  });
-
-  function serializableQueryParamsToEntries(
-    params: SerializableQueryParams
-  ): [string, string][] {
-    return params.map(({key, value}) => [key, value]);
-  }
+  return currentlyOverriddenQueryParams
+    .map((queryParamOverride) => {
+      return currentQueryParams[queryParamOverride].map((value) => ({
+        key: queryParamOverride,
+        value,
+      }));
+    })
+    .flat();
 }
diff --git a/tensorboard/webapp/routes/feature_flag_serializer_test.ts b/tensorboard/webapp/routes/feature_flag_serializer_test.ts
@@ -59,9 +59,7 @@ describe('feature flag serializer', () => {
     });
 
     it('persists flag states overridden by query params', async () => {
-      spyOn(TEST_ONLY.utils, 'getSearch').and.returnValue(
-        '?darkMode=true'
-      );
+      spyOn(TEST_ONLY.utils, 'getSearch').and.returnValue('?darkMode=true');
       console.log(location.getSearch());
       const queryParams = getOverriddenFeatureFlagStates(
         FeatureFlagMetadataMap as Record<

diff --git a/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source.ts b/tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source.ts
@@ -59,20 +59,17 @@ export class QueryParamsFeatureFlagDataSource
     if (!queryParamOverride || !params.has(queryParamOverride)) {
       return null;
     }
-    const paramValues: T[] = this.queryParams
-      .getParams()
-      .getAll(queryParamOverride)
-      .map((value) => {
-        if (value === '' && flagMetadata.defaultValue !== undefined) {
-          return flagMetadata.defaultValue;
-        }
-        return flagMetadata.parseValue(value);
-      });
+    const paramValues: T[] = params.getAll(queryParamOverride).map((value) => {
+      if (value === '' && flagMetadata.defaultValue !== undefined) {
+        return flagMetadata.defaultValue;
+      }
+      return flagMetadata.parseValue(value);
+    });
     if (!paramValues.length) {
       return null;
     }
 
-    return flagMetadata.isArray ? paramValues : paramValues[0];
+    return flagMetadata.isArray ? paramValues.flat() : paramValues[0];
   }
 
   protected getPartialFeaturesFromMediaQuery(): {