trying to fix some tests

tensorflow · Jul 15, 2022 · bc8352b · bc8352b
1 parent a7dde8c
commit bc8352b
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 88 deletions.
diff --git a/tensorboard/webapp/app_routing/location.ts b/tensorboard/webapp/app_routing/location.ts
@@ -48,6 +48,9 @@ const utils = {
   getHref() {
     return window.location.href;
   },
+  getSearch() {
+    return window.location.search;
+  }
 };
 
 @Injectable()
@@ -57,7 +60,7 @@ export class Location implements LocationInterface {
   }
 
   getSearch(): SerializableQueryParams {
-    const searchParams = new URLSearchParams(window.location.search);
+    const searchParams = new URLSearchParams(utils.getSearch());
     const serializableSearchParams: SerializableQueryParams = [];
 
     // URLSearchParams is a Iterable but TypeScript does not know about that.

diff --git a/tensorboard/webapp/feature_flag/store/feature_flag_metadata.ts b/tensorboard/webapp/feature_flag/store/feature_flag_metadata.ts
@@ -31,6 +31,7 @@ export type FeatureFlagType = BaseFeatureFlagType | Array<BaseFeatureFlagType>;
 
 export type FeatureFlagMetadata<T> = {
   displayName: string;
+  defaultValue?: T;
   queryParamOverride?: string;
   parseValue: (str: string) => T;
   isArray?: boolean;
@@ -106,6 +107,8 @@ export const FeatureFlagMetadataMap: {
   },
   inColab: {
     displayName: 'inColab',
+    defaultValue: false,
+    queryParamOverride: 'tensorboardColab',
     parseValue: parseBoolean,
   },
   metricsImageSupportEnabled: {

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, Observable} from 'rxjs';
+import {combineLatest, of, Observable} from 'rxjs';
 import {map} from 'rxjs/operators';
 import {DeepLinkProvider} from '../app_routing/deep_link_provider';
 import {SerializableQueryParams} from '../app_routing/types';
@@ -97,12 +97,12 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
           return [{key: TAG_FILTER_KEY, value: filterText}];
         })
       ),
-      getOverriddenFeatureFlagStates(
+      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/dashboard_deeplink_provider_test.ts b/tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts
@@ -347,77 +347,6 @@ describe('core deeplink provider', () => {
     });
   });
 
-  describe('feature flag', () => {
-    it('serializes enabled experimental plugins', () => {
-      store.overrideSelector(selectors.getEnabledExperimentalPlugins, [
-        'foo',
-        'bar',
-        'baz',
-      ]);
-      store.refreshState();
-
-      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
-        {key: 'experimentalPlugin', value: 'foo'},
-        {key: 'experimentalPlugin', value: 'bar'},
-        {key: 'experimentalPlugin', value: 'baz'},
-      ]);
-    });
-
-    it('serializes enableColorGroup state', () => {
-      store.overrideSelector(selectors.getOverriddenFeatureFlags, {
-        enabledColorGroup: true,
-      });
-      store.refreshState();
-
-      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
-        {key: 'enableColorGroup', value: 'true'},
-      ]);
-
-      store.overrideSelector(selectors.getOverriddenFeatureFlags, {
-        enabledColorGroup: false,
-      });
-      store.refreshState();
-
-      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
-        {key: 'enableColorGroup', value: 'false'},
-      ]);
-
-      store.overrideSelector(selectors.getOverriddenFeatureFlags, {});
-      store.refreshState();
-
-      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
-        []
-      );
-    });
-
-    it('serializes enableColorGroupByRegex state', () => {
-      store.overrideSelector(selectors.getOverriddenFeatureFlags, {
-        enabledColorGroupByRegex: true,
-      });
-      store.refreshState();
-
-      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
-        {key: 'enableColorGroupByRegex', value: 'true'},
-      ]);
-
-      store.overrideSelector(selectors.getOverriddenFeatureFlags, {
-        enabledColorGroupByRegex: false,
-      });
-      store.refreshState();
-
-      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
-        {key: 'enableColorGroupByRegex', value: 'false'},
-      ]);
-
-      store.overrideSelector(selectors.getOverriddenFeatureFlags, {});
-      store.refreshState();
-
-      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
-        []
-      );
-    });
-  });
-
   describe('runs', () => {
     describe('color group', () => {
       it('does not put state in the URL when user set color group is null', () => {

diff --git a/tensorboard/webapp/routes/feature_flag_serializer_test.ts b/tensorboard/webapp/routes/feature_flag_serializer_test.ts
@@ -13,7 +13,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 ==============================================================================*/
 import {TestBed} from '@angular/core/testing';
-import {Location} from '../app_routing/location';
+import {Location, TEST_ONLY} from '../app_routing/location';
 import {
   FeatureFlagMetadata,
   FeatureFlagMetadataMap,
@@ -23,15 +23,13 @@ import {getOverriddenFeatureFlagStates} from './feature_flag_serializer';
 
 describe('feature flag serializer', () => {
   let location: Location;
-  let getSearchSpy: jasmine.Spy;
 
   beforeEach(async () => {
     await TestBed.configureTestingModule({
-      providers: [],
+      providers: [Location],
     }).compileComponents();
 
     location = TestBed.inject(Location);
-    getSearchSpy = spyOn(location, 'getSearch').and.returnValue([]);
   });
 
   describe('getOverriddenFeatureFlagStates', () => {
@@ -45,23 +43,34 @@ describe('feature flag serializer', () => {
       expect(queryParams.length).toEqual(0);
     });
 
-    it('persists values of enabled experimental plugins', () => {});
+    it('persists values of enabled experimental plugins', () => {
+      spyOn(TEST_ONLY.utils, 'getSearch').and.returnValue(
+        '?experimentalPlugin=0&experimentalPlugin=1&experimentalPlugin=2'
+      );
+      const queryParams = getOverriddenFeatureFlagStates(
+        FeatureFlagMetadataMap as Record<
+          string,
+          FeatureFlagMetadata<FeatureFlagType>
+        >
+      );
+      expect(queryParams.length).toEqual(3);
+      expect(queryParams[0].key).toEqual('experimentalPlugin');
+      expect(queryParams[0].value).toEqual('0');
+    });
 
     it('persists flag states overridden by query params', async () => {
-      getSearchSpy = spyOn(location, 'getSearch').and.returnValue([
-        {
-          key: 'defaultEnableDarkMode',
-          value: 'true',
-        },
-      ]);
+      spyOn(TEST_ONLY.utils, 'getSearch').and.returnValue(
+        '?darkMode=true'
+      );
+      console.log(location.getSearch());
       const queryParams = getOverriddenFeatureFlagStates(
         FeatureFlagMetadataMap as Record<
           string,
           FeatureFlagMetadata<FeatureFlagType>
         >
       );
       expect(queryParams.length).toEqual(1);
-      expect(queryParams[0].key).toEqual('defaultEnableDarkMode');
+      expect(queryParams[0].key).toEqual('darkMode');
       expect(queryParams[0].value).toEqual('true');
     });
   });

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
@@ -62,7 +62,12 @@ export class QueryParamsFeatureFlagDataSource
     const paramValues: T[] = this.queryParams
       .getParams()
       .getAll(queryParamOverride)
-      .map(flagMetadata.parseValue);
+      .map((value) => {
+        if (value === '' && flagMetadata.defaultValue !== undefined) {
+          return flagMetadata.defaultValue;
+        }
+        return flagMetadata.parseValue(value);
+      });
     if (!paramValues.length) {
       return null;
     }