Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce hover tests and context menu #9

Merged
merged 2 commits into from
May 14, 2024

Conversation

haydar-metin
Copy link
Contributor

@haydar-metin haydar-metin commented Mar 15, 2024

What it does

  • Introduces hover tests and a basic context menu PO.
  • Makes the other tests more defensive by waiting if the specific elements are visible (e.g., by using waitForCreation)
  • Increased the timeout for VSCode and Theia - they need more time sometimes.
  • Introduce inverse of skipIntegration called skipNonIntegration, which does the opposite.
  • The newest version of VSCode changed the DOM of the file explorer - fixes it.

Works on eclipse-glsp/glsp#1287

How to test

Tested with GLSP v2.1.1.

  • Run all tests. yarn test.

Follow-ups

Bug found

examples/workflow-test/tests/features/validation/marker.spec.ts

  • The marker › should be still visible after resizing failing for all integrations.

Steps:

Peek 2024-03-15 13-09

Changelog

  • This PR should be mentioned in the changelog
  • This PR introduces a breaking change (if yes, provide more details below for the changelog and the migration guide)

Renames:

  • the method deselect in GLSPGraph has been renamed to focus.

- Rename deselect to select in GLSPGraph
Copy link
Contributor

@tortmayr tortmayr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot.
Code looks good to me and works great. Only a few minor comments.

@@ -43,6 +43,14 @@ The following libraries/frameworks need to be installed on your system:
- [Node.js](https://nodejs.org/en/) `>=16.11.0`
- [Yarn](https://classic.yarnpkg.com/en/docs/install#debian-stable) `>=1.7.0`

## Min versions

- [Standalone](https://github.com/eclipse-glsp/glsp-client): v2.1.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can leave this in for now, but should remove it once glsp-playwright is part of the general glsp release train (eclipse-glsp/glsp#1267).
Once that is the case it pretty self explantory that a specific version of glsp-playwright is compatible with the exact same version of the client/integration packages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we should add instead here is the minimum required VS Code version. I assume since the newest version changed the dom for the file explorer the tests no longer with older versions right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good point. Maybe we should also lock the VS Code instance to a specific version - right now it always downloads the newest version.

examples/workflow-test/configs/project.config.ts Outdated Show resolved Hide resolved
examples/workflow-test/tests/features/hover/popup.spec.ts Outdated Show resolved Hide resolved
@haydar-metin
Copy link
Contributor Author

Thanks! We will have some failing tests as the latest released version of GLSP (2.1.1) doesn't have the fixes to the bugs found here. I will introduce in a separate PR a script that will setup the different repositories automatically, so that we can work with the latest version in master.

@tortmayr
Copy link
Contributor

👍🏼 Could you pleas list the tests which are expected to fail for now?

@haydar-metin
Copy link
Contributor Author

haydar-metin commented Apr 18, 2024

 3 failed
   [standalone] › ../../tests/features/validation/marker.spec.ts:50:9 › The marker › should be still visible after resizing 
   [theia] › ../../tests/features/validation/marker.spec.ts:50:9 › The marker › should be still visible after resizing 
   [vscode] › ../../tests/features/validation/marker.spec.ts:50:9 › The marker › should be still visible after resizing 

For GLSP-Client 2.1.1

@tortmayr tortmayr self-requested a review May 14, 2024 17:32
Copy link
Contributor

@tortmayr tortmayr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. LGTM!

@tortmayr tortmayr merged commit 78741db into eclipse-glsp:main May 14, 2024
3 checks passed
@haydar-metin haydar-metin deleted the tests/hover branch May 15, 2024 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants