groups / dynamic groups bug fixes, fix e2e

.github/workflows/e2e.yml

@@ -19,8 +19,19 @@ jobs:
 
       - name: Set up Python 3.8
         uses: actions/setup-python@v2
+        id: pip-cache
         with:
           python-version: 3.8
+          cache: "pip"
+          cache-dependency-path: |
+            requirements/common.txt
+            requirements/github.txt
+            requirements/test.txt
+
+      - name: Install requirements
+        if: steps.pip-cache.outputs.cache-hit != true
+        run: |
+          pip install -r requirements/github.txt
 
       - name: Install dependencies
         run: |
@@ -59,15 +70,17 @@ jobs:
           FIFTYONE_DATABASE_NAME=cypress python ../fiftyone/server/main.py --address 0.0.0.0 --port 8787 &
           yarn install
           yarn start
-
         working-directory: ./e2e
+
       - name: Upload videos
+        if: always()
         uses: actions/upload-artifact@v2
         with:
           name: videos
           path: e2e/cypress/videos
 
       - name: Upload snapshots
+        if: always()
         uses: actions/upload-artifact@v2
         with:
           name: snapshots

app/packages/core/src/components/Actions/Options.tsx

@@ -9,7 +9,6 @@ import * as fos from "@fiftyone/state";
 import {
   configuredSidebarModeDefault,
   groupStatistics,
-  isGroup,
   sidebarMode,
 } from "@fiftyone/state";
 import RadioGroup from "../Common/RadioGroup";
@@ -130,10 +129,12 @@ type OptionsProps = {
 };
 
 const Options = ({ modal, bounds }: OptionsProps) => {
-  const group = useRecoilValue(isGroup);
+  const isGroup = useRecoilValue(fos.isGroup);
+  const isDynamicGroup = useRecoilValue(fos.isDynamicGroup);
+
   return (
     <Popout modal={modal} bounds={bounds}>
-      {group && <GroupStatistics modal={modal} />}
+      {isGroup && !isDynamicGroup && <GroupStatistics modal={modal} />}
       <MediaFields modal={modal} />
       <Patches modal={modal} />
       {!modal && <SidebarMode modal={modal} />}

app/packages/core/src/components/Modal/Group/GroupImageVideoSample.tsx

@@ -59,10 +59,11 @@ export const GroupImageVideoSample: React.FC<{
   const allSlices = useRecoilValue(fos.groupSlices);
   const altSlice = useMemo(() => {
     if (
-      sample._media_type !== "point-cloud" ||
+      sample?._media_type !== "point-cloud" ||
       currentModalSlice !== sample.group.name
-    )
+    ) {
       return undefined;
+    }
 
     if (currentModalSlice === defaultSlice) {
       return allSlices.find((s) => s !== defaultSlice);

app/packages/core/src/components/Modal/Looker.tsx

@@ -88,12 +88,13 @@ const Looker = ({
     if (propsSampleData) {
       return {
         ...modalSampleData,
+        urls,
         sample: propsSampleData,
       };
     }
 
     return modalSampleData;
-  }, [propsSampleData, modalSampleData]);
+  }, [propsSampleData, modalSampleData, urls]);
 
   const { sample } = sampleData;
 

app/packages/looker/src/elements/pcd.ts

@@ -34,7 +34,15 @@ export class PcdElement extends BaseElement<PcdState, HTMLImageElement> {
 
   renderSelf({ config: { src } }: Readonly<PcdState>) {
     if (this.src !== src) {
-      if (src.endsWith(".pcd")) {
+      // could be either pcd or projection
+      let isPcd = false;
+
+      if (src.endsWith(".pcd") || src.split("?")[0].endsWith(".pcd")) {
+        // split("?")[0] is to remove query params, if any, from signed urls
+        isPcd = true;
+      }
+
+      if (isPcd) {
         // this means orthographic projections have not been computed, replace with a 1x1 base64 blank image
         const base64Blank =
           "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4AWNgYGD4DwABDQF/w6a5YwAAAABJRU5ErkJggg==";

app/packages/state/src/recoil/groups.ts

@@ -397,7 +397,13 @@ export const groupSample = selectorFamily<SampleData, SliceName>({
         return sample;
       }
 
-      return get(groupSampleQuery(sliceName));
+      const fallbackSample = get(groupSampleQuery(sliceName));
+
+      if (fallbackSample?.sample) {
+        return fallbackSample;
+      }
+
+      return sample;
     },
 });
 

e2e/cypress.config.ts

@@ -15,13 +15,13 @@ export default defineConfig({
     /** end: env for cypress-visual-regression */
   },
   e2e: {
-    video: process.env.CI === "true",
+    video: false,
     baseUrl: DEFAULT_APP_ADDRESS,
     videoUploadOnPasses: false,
     experimentalInteractiveRunEvents: true,
-    // retry once on test failure to account for random errors
+    // retry once on test failure to account for random errors in CI
     // note: this is a global config, this can be configured per-test as well
-    retries: 1,
+    retries: process.env.CI ? 1 : 0,
     setupNodeEvents(on, config) {
       getCompareSnapshotsPlugin(on, config);
 

e2e/cypress/e2e/dynamic-groups/dynamic-groups.cy.ts

@@ -63,8 +63,7 @@ const navigateDynamicGroupsModal = (ordered: boolean) => {
     const sceneIds = new Set(["0", "1", "2", "3", "4", "5", "6", "7"]);
 
     for (let i = 0; i < sceneIds.size; i++) {
-      // wait for flashlight to load, otherwise scene_id will be incorrect
-      cy.waitForLookerToRender();
+      cy.get("[data-cy=sidebar-entry-id]").should("be.visible");
       cy.get("[data-cy=sidebar-entry-scene_id]").then((sceneId) => {
         const sceneIdText = sceneId.text();
         const sceneIdNumber = Number(sceneIdText);
@@ -106,6 +105,7 @@ const navigateDynamicGroupsModal = (ordered: boolean) => {
         // check if random access works
         cy.get("[data-cy=dynamic-group-pagination-bar-input]")
           .clear()
+          .wait(10)
           .type("22");
 
         verifySceneIdTimestamp(sceneIdNumber, 21, ordered);
@@ -187,15 +187,7 @@ describe("dynamic groups", () => {
   context("folding quickstart-groups (media type = group) by scene_id", () => {
     before(() => {
       cy.executePythonFixture("dynamic-groups/dynamic-groups.cy.py");
-    });
-
-    beforeEach(() => {
       cy.waitForGridToBeVisible("quickstart-groups-dynamic");
-      cy.get("[data-cy=entry-counts").should("have.text", "200 groups");
-    });
-
-    afterEach(() => {
-      cy.clearViewStages();
     });
 
     it("should show valid candidates for group-by keys", () => {
@@ -207,34 +199,47 @@ describe("dynamic groups", () => {
       ]);
     });
 
-    context("unordered", () => {
-      it("should create dynamic groups based on scene_id", () => {
-        cy.get("[data-cy=entry-counts").should("have.text", "200 groups");
+    ["left", "right", "pcd"].forEach((slice) => {
+      context(`unordered with slice = ${slice}`, () => {
+        before(() => {
+          cy.waitForGridToBeVisible("quickstart-groups-dynamic");
+          cy.clearViewStages();
 
-        // todo: investigate, "first()" is used to suppress this flakiness;
-        // sometimes an extra element is rendered and the test fails
-        cy.get("[data-cy=dynamic-group-pill-button]").click();
-        cy.get("[data-cy=order-by-selector]").should("not.exist");
-        cy.get("[data-cy=group-by-selector]").click();
-        cy.get("[data-cy=selector-result-scene_id]").click();
+          cy.get("[data-cy=entry-counts").should("have.text", "200 groups");
+          // todo: investigate, "first()" is used to suppress this flakiness;
+          // sometimes an extra element is rendered and the test fails
+          cy.get("[data-cy=dynamic-group-pill-button]").click();
+          cy.get("[data-cy=order-by-selector]").should("not.exist");
+          cy.get("[data-cy=group-by-selector]").click();
+          cy.get("[data-cy=selector-result-scene_id]").click();
 
-        cy.get("[data-cy=dynamic-group-btn-submit]").click();
+          cy.get("[data-cy=dynamic-group-btn-submit]").click();
 
-        cy.get("[data-cy=entry-counts").should("have.text", "8 groups");
+          cy.get("[data-cy=entry-counts").should("have.text", "8 groups");
 
-        cy.get("[data-cy=checkbox-scene_id]").click();
-        cy.get("[data-cy=checkbox-timestamp]").click();
+          cy.get("[data-cy=checkbox-scene_id]").click();
+          cy.get("[data-cy=checkbox-timestamp]").click();
 
-        cy.get("[data-cy=looker]").should("have.length", 8);
+          cy.get("[data-cy=looker]").should("have.length", 8);
+        });
 
-        cy.get("[data-cy=looker]").first().click();
+        beforeEach(() => {
+          cy.get("[data-cy=selector-slice]").first().click();
+          cy.get(`[data-cy=selector-result-${slice}]`).click();
+          cy.get("[data-cy=looker]").first().click();
+        });
 
-        navigateDynamicGroupsModal(false);
+        it("should create dynamic groups based on scene_id", () => {
+          navigateDynamicGroupsModal(false);
+        });
       });
     });
 
     context("ordered", () => {
       it("should create dynamic groups based on scene_id and ordered by timestamp", () => {
+        cy.waitForGridToBeVisible();
+        cy.clearViewStages();
+
         // todo: investigate, "first()" is used to suppress this flakiness;
         // sometimes an extra element is rendered and the test fails
         cy.get("[data-cy=dynamic-group-pill-button]").click();

e2e/cypress/support/commands.ts

@@ -50,10 +50,39 @@ Cypress.Commands.add(
 );
 
 Cypress.Commands.add("clearViewStages", () => {
-  // chaining with root in case modal is open and ctx is within modal
-  cy.root().find("[data-cy=btn-clear-view-bar]").click();
+  const clear = () => {
+    // chaining with root in case modal is open and ctx is within modal
+    cy.root().find("[data-cy=btn-clear-view-bar]").click();
+    // unfortunately, can take long.
+    // todo: emit an event and make it more deterministic.
+    cy.wait(1000);
+  };
+
+  const checkIfStagesAreClear = (attemptCount: number) => {
+    // check if view stages is clear, if not, call clear() again
+    cy.get("[data-cy=view-stage-container]").then((elements) => {
+      const isStagesClear =
+        elements.length === 1 && elements.text().includes("add stage");
+      if (!isStagesClear && attemptCount < 3) {
+        console.log(`view stages not clear, (attempt ${attemptCount})`);
+        clear();
+        // call recursively until view stages are clear or attempt limit reached
+        checkIfStagesAreClear(attemptCount + 1);
+      }
+
+      if (!isStagesClear && attemptCount >= 3) {
+        throw new Error("view stages not clear after 3 attempts");
+      }
+    });
+  };
+
+  clear();
+  // xstate lib randomly errors out sometimes, so we need to check if stages are clear
+  // check custom exception handling in e2e.ts for more info
+  checkIfStagesAreClear(1);
 });
 
 Cypress.Commands.add("consoleLog", (message) => {
+  console.log(message);
   cy.task("logTask", message);
 });

e2e/cypress/support/e2e.ts

@@ -3,18 +3,47 @@ import compareSnapshotCommand from "cypress-visual-regression/dist/command";
 
 import "./commands";
 
+Cypress.on("uncaught:exception", (err, runnable, promise) => {
+  // returning false here prevents Cypress from failing the test
+  // don't invoke other cypress commands here, otherwise you'll get red herrings that make debugging hard
+  // https://github.com/cypress-io/cypress/issues/1217#issuecomment-398461289
+
+  // Unhandled promise rejections will cause seemingly random errors that
+  // are very hard to debug.
+  if (promise) {
+    console.log("Unhandled promise rejection.");
+    console.log(promise);
+    // If false is returned the exception will be ignored and won't
+    // cause the test to fail.
+    return false;
+  }
+
+  if (
+    // occasionally get this error when clearing the view stages
+    err.message.includes("Unable to evaluate guard") ||
+    // sometimes ResizeObserver fails with the message
+    // `ResizeObserver loop limit exceeded`
+    // Seems to occur for us in Cypress but never in browser in normal use
+    err.message.includes("ResizeObserver") ||
+    // occasionally get this error from react-three-fiber library
+    err.message.includes(
+      "Cannot read properties of null (reading 'addEventListener')"
+    )
+  ) {
+    console.log(`caught known error: ${err.message}`);
+    return false;
+  }
+
+  // fail in all other cases
+  return true;
+});
+
 compareSnapshotCommand({
   capture: "viewport",
   overwrite: true,
   errorThreshold: 0,
 });
 
-Cypress.on("uncaught:exception", (err, runnable) => {
-  // returning false here prevents Cypress from failing the test
-  // if there're certain exceptions we want to ignore, we can add them here
-  return true;
-});
-
 // delete all datasets before each spec runs
 // todo: investigate, this is flaky, sometimes singleton state doesn't reflect dataset deletion
 before(() => {

e2e/package.json

@@ -18,7 +18,7 @@
         "@types/wait-on": "^5.3.1",
         "body-parser": "^1.20.2",
         "cors": "^2.8.5",
-        "cypress": "^12.8.0",
+        "cypress": "^12.14.0",
         "cypress-promise": "^1.1.0",
         "cypress-visual-regression": "^3.0.0",
         "dotenv": "^16.0.3",

e2e/yarn.lock

@@ -571,6 +571,11 @@ commander@^5.1.0:
   resolved "https://registry.yarnpkg.com/commander/-/commander-5.1.0.tgz#46abbd1652f8e059bddaef99bbdcb2ad9cf179ae"
   integrity sha512-P0CysNDQ7rtVw4QIQtm+MRxV66vKFSvlsQvGYXZWR3qFU0jlMKHZZZgw8e+8DSah4UDKMqnknRDQz+xuQXQ/Zg==
 
+commander@^6.2.1:
+  version "6.2.1"
+  resolved "https://registry.yarnpkg.com/commander/-/commander-6.2.1.tgz#0792eb682dfbc325999bb2b84fddddba110ac73c"
+  integrity sha512-U7VdrJFnJgo4xjrHpTzu0yrHPGImdsmD95ZlgYSEajAn2JKzDhDTPG9kBTefmObL2w/ngeZnilk+OV9CG3d7UA==
+
 common-tags@^1.8.0:
   version "1.8.2"
   resolved "https://registry.yarnpkg.com/common-tags/-/common-tags-1.8.2.tgz#94ebb3c076d26032745fd54face7f688ef5ac9c6"
@@ -697,10 +702,10 @@ cypress@*:
     untildify "^4.0.0"
     yauzl "^2.10.0"
 
-cypress@^12.8.0:
-  version "12.8.0"
-  resolved "https://registry.yarnpkg.com/cypress/-/cypress-12.8.0.tgz#d31b86ad7b39678b4fa20892f60a8ed58b271729"
-  integrity sha512-+zblUeRIesOwpZ/E0HT0YE19STwbZy/ySJZIW6BHxX/eVX258gs4t9hP3sE4Opw9y0RfhJeoE8oa1wF/WhIb0A==
+cypress@^12.14.0:
+  version "12.14.0"
+  resolved "https://registry.yarnpkg.com/cypress/-/cypress-12.14.0.tgz#37a19b85f5e9d881995e9fee1ddf41b3d3a623dd"
+  integrity sha512-HiLIXKXZaIT1RT7sw1sVPt+qKtis3uYNm6KwC4qoYjabwLKaqZlyS/P+uVvvlBNcHIwL/BC6nQZajpbUd7hOgQ==
   dependencies:
     "@cypress/request" "^2.88.10"
     "@cypress/xvfb" "^1.2.4"
@@ -716,7 +721,7 @@ cypress@^12.8.0:
     check-more-types "^2.24.0"
     cli-cursor "^3.1.0"
     cli-table3 "~0.6.1"
-    commander "^5.1.0"
+    commander "^6.2.1"
     common-tags "^1.8.0"
     dayjs "^1.10.4"
     debug "^4.3.4"
@@ -734,7 +739,7 @@ cypress@^12.8.0:
     listr2 "^3.8.3"
     lodash "^4.17.21"
     log-symbols "^4.0.0"
-    minimist "^1.2.6"
+    minimist "^1.2.8"
     ospath "^1.2.2"
     pretty-bytes "^5.6.0"
     proxy-from-env "1.0.0"
@@ -1445,7 +1450,7 @@ minimist@^1.2.6:
   resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.6.tgz#8637a5b759ea0d6e98702cfb3a9283323c93af44"
   integrity sha512-Jsjnk4bw3YJqYzbdyBiNsPWHPfO++UGG749Cxs6peCu5Xg4nrena6OVxOYxrQTqww0Jmwt+Ref8rggumkTLz9Q==
 
-minimist@^1.2.7:
+minimist@^1.2.7, minimist@^1.2.8:
   version "1.2.8"
   resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.8.tgz#c1a464e7693302e082a075cee0c057741ac4772c"
   integrity sha512-2yyAR8qBkN3YuheJanUpWC5U3bb5osDywNB8RzDVlDwDHbocAJveqqj1u8+SVD7jkWT4yvsHCpWqqWqAxb0zCA==