Browse Source

Revert visual regression (#10376)

* Revert visual regression

Correct regression introduced in #10127
Fixes #10375

* Update reference screenshots

* Generate new reference candidate if UI test failed

* Add missing new references

* Refactor tests

* Fix test

* Fix doc

* Push new reference for tag in toc

* Update docs/source/developer/contributing.rst

Co-authored-by: Jeremy Tuloup <jeremy.tuloup@gmail.com>
Frédéric Collonval 3 years ago
parent
commit
3a3b4013c1
32 changed files with 119 additions and 74 deletions
  1. 82 65
      .github/workflows/ui-tests.yml
  2. 7 0
      docs/source/developer/contributing.rst
  3. 1 1
      packages/cells/style/collapser.css
  4. BIN
      ui-tests/reference-output/screenshots/notebook_create_dark_theme.png
  5. BIN
      ui-tests/reference-output/screenshots/notebook_create_run_cells.png
  6. BIN
      ui-tests/reference-output/screenshots/notebook_edit_copy_paste_cell.png
  7. BIN
      ui-tests/reference-output/screenshots/notebook_edit_cut_paste_cell.png
  8. BIN
      ui-tests/reference-output/screenshots/notebook_edit_delete_cell.png
  9. BIN
      ui-tests/reference-output/screenshots/notebook_edit_deselect_all_cells.png
  10. BIN
      ui-tests/reference-output/screenshots/notebook_edit_merge_cells.png
  11. BIN
      ui-tests/reference-output/screenshots/notebook_edit_move_cell_down.png
  12. BIN
      ui-tests/reference-output/screenshots/notebook_edit_move_cell_up.png
  13. BIN
      ui-tests/reference-output/screenshots/notebook_edit_paste_replace_cell.png
  14. BIN
      ui-tests/reference-output/screenshots/notebook_edit_select_all_cells.png
  15. BIN
      ui-tests/reference-output/screenshots/notebook_edit_split_cell.png
  16. BIN
      ui-tests/reference-output/screenshots/notebook_run_page_0.png
  17. BIN
      ui-tests/reference-output/screenshots/notebook_toolbar_change_to_markdown.png
  18. BIN
      ui-tests/reference-output/screenshots/notebook_toolbar_copy_paste_cell.png
  19. BIN
      ui-tests/reference-output/screenshots/notebook_toolbar_cut_cell.png
  20. BIN
      ui-tests/reference-output/screenshots/notebook_toolbar_delete_cell.png
  21. BIN
      ui-tests/reference-output/screenshots/notebook_toolbar_insert_cells.png
  22. BIN
      ui-tests/reference-output/screenshots/notebook_toolbar_paste_cell.png
  23. BIN
      ui-tests/reference-output/screenshots/notebook_toolbar_re_run_cell.png
  24. BIN
      ui-tests/reference-output/screenshots/notebook_toolbar_run_cell.png
  25. BIN
      ui-tests/reference-output/screenshots/toc_show_tags.png
  26. BIN
      ui-tests/reference-output/screenshots/toc_toc_panel.png
  27. BIN
      ui-tests/reference-output/screenshots/toc_toggle_code.png
  28. BIN
      ui-tests/reference-output/screenshots/toc_toggle_markdown.png
  29. BIN
      ui-tests/reference-output/screenshots/toc_toggle_numbered_list.png
  30. BIN
      ui-tests/reference-output/screenshots/toc_toggle_tag_1.png
  31. BIN
      ui-tests/reference-output/screenshots/toc_toggle_tag_2.png
  32. 29 8
      ui-tests/tests/toc.test.ts

+ 82 - 65
.github/workflows/ui-tests.yml

@@ -13,74 +13,91 @@ jobs:
       fail-fast: false
 
     steps:
-    - uses: actions/checkout@v2
-    - name: Set up Python
-      uses: actions/setup-python@v1
-      with:
-        python-version: ${{ matrix.python }}
+      - uses: actions/checkout@v2
+      - name: Set up Python
+        uses: actions/setup-python@v1
+        with:
+          python-version: ${{ matrix.python }}
 
-    - name: Set up Node
-      uses: actions/setup-node@v1
-      with:
-        node-version: '12.x'
+      - name: Set up Node
+        uses: actions/setup-node@v1
+        with:
+          node-version: '12.x'
 
-    - name: Cache pip on Linux
-      uses: actions/cache@v2
-      if: startsWith(runner.os, 'Linux')
-      with:
-        path: ~/.cache/pip
-        key: ${{ runner.os }}-pip-${{ matrix.python }}-${{ hashFiles('**/requirements.txt', 'setup.py') }}
-        restore-keys: |
-          ${{ runner.os }}-pip-${{ matrix.python }}
+      - name: Cache pip on Linux
+        uses: actions/cache@v2
+        if: startsWith(runner.os, 'Linux')
+        with:
+          path: ~/.cache/pip
+          key: ${{ runner.os }}-pip-${{ matrix.python }}-${{ hashFiles('**/requirements.txt', 'setup.py') }}
+          restore-keys: |
+            ${{ runner.os }}-pip-${{ matrix.python }}
 
-    - name: Get yarn cache directory path
-      id: yarn-cache-dir-path
-      run: echo "::set-output name=dir::$(yarn cache dir)"
-    - name: Cache yarn
-      uses: actions/cache@v2
-      id: yarn-cache # use this to check for `cache-hit` (`steps.yarn-cache.outputs.cache-hit != 'true'`)
-      with:
-        path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
-        key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
-        restore-keys: |
-          ${{ runner.os }}-yarn-
+      - name: Get yarn cache directory path
+        id: yarn-cache-dir-path
+        run: echo "::set-output name=dir::$(yarn cache dir)"
+      - name: Cache yarn
+        uses: actions/cache@v2
+        id: yarn-cache # use this to check for `cache-hit` (`steps.yarn-cache.outputs.cache-hit != 'true'`)
+        with:
+          path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
+          key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-yarn-
 
-    - name: Install dependencies
-      env:
-        GROUP: ${{ matrix.group }}
-      run: |
-        bash ./scripts/ci_install.sh
-        jlpm
-        jlpm build
-        jupyter lab build
+      - name: Install dependencies
+        env:
+          GROUP: ${{ matrix.group }}
+        run: |
+          bash ./scripts/ci_install.sh
+          jlpm
+          jlpm build
+          jupyter lab build
 
-    - name: Install Galata
-      run: |
-        cd ui-tests
-        jlpm install --frozen-lockfile
-    
-    - name: Launch JupyterLab
-      run: |
-        cd ui-tests
-        mkdir jlab_root
-        jlpm run start-jlab:detached
-    
-    - name: Wait for JupyterLab
-      uses: ifaxity/wait-on-action@v1
-      with:
-        resource: http-get://localhost:8888/api
-        timeout: 20000
-    
-    - name: Run UI Tests
-      run: |
-        cd ui-tests
-        jlpm run test
-        rm -rf jlab_root
+      - name: Install Galata
+        run: |
+          cd ui-tests
+          jlpm install --frozen-lockfile
 
-    - name: Upload UI Test artifacts
-      if: always()
-      uses: actions/upload-artifact@v2
-      with:
-        name: ui-test-output
-        path: |
-          ui-tests/test-output
+      - name: Launch JupyterLab
+        run: |
+          cd ui-tests
+          mkdir jlab_root
+          jlpm run start-jlab:detached
+
+      - name: Wait for JupyterLab
+        uses: ifaxity/wait-on-action@v1
+        with:
+          resource: http-get://localhost:8888/api
+          timeout: 20000
+
+      - name: Run UI Tests
+        run: |
+          cd ui-tests
+          jlpm run test
+          rm -rf jlab_root
+
+      - name: Upload UI Test artifacts
+        if: always()
+        uses: actions/upload-artifact@v2
+        with:
+          name: ui-test-output
+          path: |
+            ui-tests/test-output
+
+      - name: Run UI Tests
+        if: ${{ failure() }}
+        run: |
+          cd ui-tests
+          [ -d "jlab_root" ] && rm -rf jlab_root
+          mkdir jlab_root
+          jlpm run test:create-references
+          rm -rf jlab_root
+
+      - name: Upload UI Test new reference artifacts
+        if: ${{ failure() }}
+        uses: actions/upload-artifact@v2
+        with:
+          name: ui-test-new-reference
+          path: |
+            ui-tests/test-output/test/screenshots/*.png

+ 7 - 0
docs/source/developer/contributing.rst

@@ -544,6 +544,13 @@ Main reasons for UI test failures are:
 
 For more information on UI Testing, please read the `UI Testing developer documentation <.https://github.com/jupyterlab/jupyterlab/blob/master/ui-tests/README.md>`__ and `Galata documentation <https://github.com/jupyterlab/galata/blob/main/README.md>`__.
 
+Good Practices for Integration tests
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Here are some good practices to follow when writing integration tests:
+
+- Don't compare multiple screenshots in the same test; if the first comparison breaks, it will require running multiple times the CI workflow to fix all tests.
+
 Contributing to the debugger front-end
 --------------------------------------
 

+ 1 - 1
packages/cells/style/collapser.css

@@ -4,7 +4,7 @@
 |----------------------------------------------------------------------------*/
 
 .jp-Collapser {
-  display: 0 0 var(--jp-cell-collapser-width);
+  flex: 0 0 var(--jp-cell-collapser-width);
   padding: 0px;
   margin: 0px;
   border: none;

BIN
ui-tests/reference-output/screenshots/notebook_create_dark_theme.png


BIN
ui-tests/reference-output/screenshots/notebook_create_run_cells.png


BIN
ui-tests/reference-output/screenshots/notebook_edit_copy_paste_cell.png


BIN
ui-tests/reference-output/screenshots/notebook_edit_cut_paste_cell.png


BIN
ui-tests/reference-output/screenshots/notebook_edit_delete_cell.png


BIN
ui-tests/reference-output/screenshots/notebook_edit_deselect_all_cells.png


BIN
ui-tests/reference-output/screenshots/notebook_edit_merge_cells.png


BIN
ui-tests/reference-output/screenshots/notebook_edit_move_cell_down.png


BIN
ui-tests/reference-output/screenshots/notebook_edit_move_cell_up.png


BIN
ui-tests/reference-output/screenshots/notebook_edit_paste_replace_cell.png


BIN
ui-tests/reference-output/screenshots/notebook_edit_select_all_cells.png


BIN
ui-tests/reference-output/screenshots/notebook_edit_split_cell.png


BIN
ui-tests/reference-output/screenshots/notebook_run_page_0.png


BIN
ui-tests/reference-output/screenshots/notebook_toolbar_change_to_markdown.png


BIN
ui-tests/reference-output/screenshots/notebook_toolbar_copy_paste_cell.png


BIN
ui-tests/reference-output/screenshots/notebook_toolbar_cut_cell.png


BIN
ui-tests/reference-output/screenshots/notebook_toolbar_delete_cell.png


BIN
ui-tests/reference-output/screenshots/notebook_toolbar_insert_cells.png


BIN
ui-tests/reference-output/screenshots/notebook_toolbar_paste_cell.png


BIN
ui-tests/reference-output/screenshots/notebook_toolbar_re_run_cell.png


BIN
ui-tests/reference-output/screenshots/notebook_toolbar_run_cell.png


BIN
ui-tests/reference-output/screenshots/toc_show_tags.png


BIN
ui-tests/reference-output/screenshots/toc_toc_panel.png


BIN
ui-tests/reference-output/screenshots/toc_toggle_code.png


BIN
ui-tests/reference-output/screenshots/toc_toggle_markdown.png


BIN
ui-tests/reference-output/screenshots/toc_toggle_numbered_list.png


BIN
ui-tests/reference-output/screenshots/toc_toggle_tag_1.png


BIN
ui-tests/reference-output/screenshots/toc_toggle_tag_2.png


+ 29 - 8
ui-tests/tests/toc.test.ts

@@ -106,31 +106,43 @@ describe('Table of Contents', () => {
     expect(await galata.capture.compareScreenshot(imageName)).toBe('same');
   });
 
-  test('Toggle code-markdown-list', async () => {
+  test('Toggle code', async () => {
     const tocPanel = await galata.sidebar.getContentPanel('left');
     const toolbarButtons = await tocPanel.$$('.toc-toolbar .toc-toolbar-icon');
     expect(toolbarButtons.length).toBe(4);
 
-    let imageName = 'toggle-code';
+    const imageName = 'toggle-code';
     await toolbarButtons[0].click();
     await galata.capture.screenshot(imageName, tocPanel);
     expect(await galata.capture.compareScreenshot(imageName)).toBe('same');
     await toolbarButtons[0].click();
+  });
+
+  test('Toggle markdown', async () => {
+    const tocPanel = await galata.sidebar.getContentPanel('left');
+    const toolbarButtons = await tocPanel.$$('.toc-toolbar .toc-toolbar-icon');
+    expect(toolbarButtons.length).toBe(4);
 
-    imageName = 'toggle-markdown';
+    const imageName = 'toggle-markdown';
     await toolbarButtons[1].click();
     await galata.capture.screenshot(imageName, tocPanel);
     expect(await galata.capture.compareScreenshot(imageName)).toBe('same');
     await toolbarButtons[1].click();
+  });
 
-    imageName = 'toggle-numbered-list';
+  test('Toggle list', async () => {
+    const tocPanel = await galata.sidebar.getContentPanel('left');
+    const toolbarButtons = await tocPanel.$$('.toc-toolbar .toc-toolbar-icon');
+    expect(toolbarButtons.length).toBe(4);
+
+    const imageName = 'toggle-numbered-list';
     await toolbarButtons[2].click();
     await galata.capture.screenshot(imageName, tocPanel);
     expect(await galata.capture.compareScreenshot(imageName)).toBe('same');
     await toolbarButtons[2].click();
   });
 
-  test('Toggle tags', async () => {
+  test('Toggle show tags', async () => {
     const tocPanel = await galata.sidebar.getContentPanel('left');
     const toolbarButtons = await tocPanel.$$('.toc-toolbar .toc-toolbar-icon');
     expect(toolbarButtons.length).toBe(4);
@@ -139,21 +151,30 @@ describe('Table of Contents', () => {
     await toolbarButtons[0].click();
     await toolbarButtons[1].click();
 
-    let imageName = 'show-tags';
+    const imageName = 'show-tags';
     await toolbarButtons[3].click();
     await galata.capture.screenshot(imageName, tocPanel);
     expect(await galata.capture.compareScreenshot(imageName)).toBe('same');
+  });
+
+  test('Toggle tag 1', async () => {
+    const tocPanel = await galata.sidebar.getContentPanel('left');
 
     const tags = await tocPanel.$$('.toc-tag');
     expect(tags.length).toBe(2);
 
-    imageName = 'toggle-tag-1';
+    const imageName = 'toggle-tag-1';
     await tags[0].click();
     await galata.capture.screenshot(imageName, tocPanel);
     expect(await galata.capture.compareScreenshot(imageName)).toBe('same');
     await tags[0].click();
+  });
+
+  test('Toggle tag 2', async () => {
+    const tocPanel = await galata.sidebar.getContentPanel('left');
+    const tags = await tocPanel.$$('.toc-tag');
 
-    imageName = 'toggle-tag-2';
+    const imageName = 'toggle-tag-2';
     await tags[1].click();
     await galata.capture.screenshot(imageName, tocPanel);
     expect(await galata.capture.compareScreenshot(imageName)).toBe('same');