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

Test a command's effect on visible ranges #160

Closed
2 tasks done
brxck opened this issue Jul 28, 2021 · 4 comments · Fixed by #539
Closed
2 tasks done

Test a command's effect on visible ranges #160

brxck opened this issue Jul 28, 2021 · 4 comments · Fixed by #539
Assignees
Labels
code quality Improvements to code quality

Comments

@brxck
Copy link
Collaborator

brxck commented Jul 28, 2021

Currently we're storing visible ranges during test case recording, but we are not asserting anything about them.

This will be interesting because visible ranges depend on the size of the editor, and because the tests will run headlessly in CI.

To do

@brxck
Copy link
Collaborator Author

brxck commented Aug 1, 2021

In headless Linux CI machines xvfb is required to run VS Code

💭 xvfb allows us to specify a screen size...

@pokey
Copy link
Member

pokey commented Aug 3, 2021

Ooh that's cool @brxck. So sounds like we might be able to do this one more easily on Linux. Then I guess we'd check OS and only check that scrolling happened on Linux, but on all OSes at least check that it didn't break? Tho one problem is that different screen size might impact navigation map because cursorless only decorates visible ranges. So it might use hats in different places if it's trying to decorate more / less tokens. In current tests all files fit on screen so we don't have that issue. So we might need to get fancier bout navigation map if we impl this one. Eg actually modify it rather than just asserting it matches our expectations. (Which seems to be flakey (#173) for some reason anyway at this point 😅)

@pokey pokey added the code quality Improvements to code quality label Aug 6, 2021
@pokey pokey mentioned this issue Aug 11, 2021
2 tasks
@pokey
Copy link
Member

pokey commented Jan 4, 2022

Thinking about this one a bit more, I don't think we need to worry bout screen size. We can test scrolling as follows. Have a file like below:

hello
there

And then say "crown trap". The "hello" should scroll off the top of the screen, and thus not be part of visible ranges

@pokey pokey added this to the 0.25.0 milestone Jan 4, 2022
@pokey pokey removed this from the On deck milestone Jan 18, 2022
@AndreasArvidsson
Copy link
Member

@pokey Should this just be one of unit test or do we want to add is functionality to the test recorder? Your easy examples above would probably work but if we validate them for every single test in the test recorder there is potential for problems of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Improvements to code quality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants