Skip to content

SKETCH-2643: Migrate squish tests to CI via playwright tests - #376

Draft
JarrettSJohnson wants to merge 1 commit into
mainfrom
squish_port
Draft

SKETCH-2643: Migrate squish tests to CI via playwright tests#376
JarrettSJohnson wants to merge 1 commit into
mainfrom
squish_port

Conversation

@JarrettSJohnson

@JarrettSJohnson JarrettSJohnson commented Jun 18, 2026

Copy link
Copy Markdown
Member

Summary

Migrates the portable parts of the Squish 2D sketcher coverage into Playwright/WASM e2e tests.

  • Adds fixture-backed import smoke coverage for historically problematic SDF/MAE structures from suite_2D_sketcher/tst_problematic_structures.
  • Ports the reusable import-format coverage from suite_2D_sketcher_new/tst_import_menu, including methylphenidate fixtures across supported text formats, InChI/SMARTS/complex-SMILES import cases, and the oversized-structure rejection path.
  • Ports the SHARED-6974 miscellaneous MAE regression structures.
  • Adds behavioral coverage for keyboard element shortcuts that can be validated through exported SMILES.
  • Extends the Playwright helpers with clearSketcher and tryImport so import sweeps can collect failures without aborting on the first thrown exception.

Notes

Maestro-specific Squish behavior was intentionally left out, including Project Table workflows, file dialogs, Maestro help/export menus, and Qt pixel-reference checks that do not map cleanly to the standalone WASM sketcher.

@JarrettSJohnson
JarrettSJohnson force-pushed the squish_port branch 2 times, most recently from 382b295 to 6901d9f Compare July 8, 2026 17:19
@JarrettSJohnson JarrettSJohnson changed the title Squish port SKETCH-2643: Migrate squish tests to CI via playwright tests Jul 8, 2026
@Sean-Seekins

Copy link
Copy Markdown

This looks promising! I just have one suggestion to make sure we go in the right direction. We should use the most recent squish tests for this conversion: https://opengrok.schrodinger.com/xref/gui_tests/priority_4/suite_molviewer/. There are "suite_2D_sketcher_new/" tests, which are for the 'new sketcher', as opposed to the 'old sketcher', which has tests in "https://opengrok.schrodinger.com/xref/gui_tests/priority_1/suite_2D_sketcher/".

So the timeline went 'old sketcher' (priority_1/suite_2D_sketcher/) -> 'new sketcher' (priority_4/suite_2D_sketcher_new/) -> 'molviewer' (priority_4/suite_molviewer/), and now we have the monomeric sketcher as well. The molviewer tests are stored in priority_4 so they don't run on automated runs (too tedious to manage those failures with image differences on different machines, so I just run locally).

If that seems confusing, I agree. It is because of the way that it is.

@JarrettSJohnson

Copy link
Copy Markdown
Member Author

This looks promising! I just have one suggestion to make sure we go in the right direction. We should use the most recent squish tests for this conversion: https://opengrok.schrodinger.com/xref/gui_tests/priority_4/suite_molviewer/. There are "suite_2D_sketcher_new/" tests, which are for the 'new sketcher', as opposed to the 'old sketcher', which has tests in "https://opengrok.schrodinger.com/xref/gui_tests/priority_1/suite_2D_sketcher/".

So the timeline went 'old sketcher' (priority_1/suite_2D_sketcher/) -> 'new sketcher' (priority_4/suite_2D_sketcher_new/) -> 'molviewer' (priority_4/suite_molviewer/), and now we have the monomeric sketcher as well. The molviewer tests are stored in priority_4 so they don't run on automated runs (too tedious to manage those failures with image differences on different machines, so I just run locally).

If that seems confusing, I agree. It is because of the way that it is.

AFAICT, these are inspired from all of the above including priority_4/suite_molviewer. Let me know if there's anything I'm missing from there. They're not all yet ported due to some inability to query atom/bond coordinates, but that's seems to be flag'd on my agent's next TODO.

Port the portable intent of the squish suites suite_2D_sketcher and
suite_2D_sketcher_new to JavaScript Playwright tests driving the standalone
WASM sketcher. Maestro-bound cases (Project Table, 2D Viewer panel, file/help
menus) and index-based interaction cases (which need an atom/bond hit-testing
binding) are out of scope.

New test files:
- problematic_structures.test.js: each SDF record / .mae structure imports
  without throwing (multi-structure SDF and .mae are split per record)
- import_formats.test.js: methylphenidate across 11 formats canonicalizes
  consistently; InChI/SMARTS/complex-SMILES import; 200-atom limit rejects
- miscellaneous.test.js: SHARED-6974 regression structures import
- keyboard_shortcuts.test.js: element key shortcuts place the matching atom

Helpers: add clearSketcher, tryImport (with getExceptionMessage decoding).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants