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

Fixes #2205. Reduce fragility of clipboard related unit tests #2206

Merged
merged 44 commits into from Jan 2, 2023

Conversation

tig
Copy link
Collaborator

@tig tig commented Nov 8, 2022

Fixes #2205

@tig
Copy link
Collaborator Author

tig commented Nov 8, 2022

Windows tests, before:
image

Windows tests, after:
image

So not significantly faster (was hoping that as a side benefit).

@tig
Copy link
Collaborator Author

tig commented Nov 8, 2022

Before on Debian:
image

After on Debian:
image

SIGNIFICANTLY FASTER

@tig tig changed the title Fixes 2205. Reduce fragility of clipboard related unit tests Fixes #2205. Reduce fragility of clipboard related unit tests Nov 8, 2022
@tig
Copy link
Collaborator Author

tig commented Nov 9, 2022

@BDisp - Man, I feel your pain. The Clipboard on WSL and Linux is a mysterious beast!

I think I made solid progress in making the unit tests more robust with this PR. Please review.

@tznind, @BDisp (or anyone else) - please pull this down locally on a Mac AND real Linux install and test it please. I don't have either.

@tznind
Copy link
Collaborator

tznind commented Nov 9, 2022

Can confirm that running Ubuntu laptop things work as expected.

Distributor ID:	Ubuntu
Description:	Ubuntu 20.04.4 LTS
Release:	20.04
Codename:	focal

I was able to copy/paste when xclip was installed. When it was not installed I was unable to copy and when I try to paste I get the text 'Clipboard Not Supported'.

@tig
Copy link
Collaborator Author

tig commented Nov 11, 2022

Alright, I think I have everything fixed except the Curses reset issue @BDisp noted.

Still trying to figure that one out...

@tig
Copy link
Collaborator Author

tig commented Nov 11, 2022

As a developer it would be nice if it were easy to tailor the libraries behaviour under such circumstances e.g. instead of outputting "Clipboard not available" it could:

  • Output nothing at all
  • Copy/Paste from an internal 'memory only' clipboard class
  • Display a modal dialog telling my users to install xclip (if on linux)

This is the correct behavior, IMO, and I just fixed a stupid mistake that makes it work this way (which is how it worked before):

  • Copy/Paste from an internal 'memory only' clipboard class

@BDisp
Copy link
Collaborator

BDisp commented Nov 11, 2022

Alright, I think I have everything fixed except the Curses reset issue @BDisp noted.

Still trying to figure that one out...

One that is occur to me is any exception when is thrown deviates the execution of the reset.

@tig tig added this to the v1.9 milestone Dec 4, 2022
@tig tig self-assigned this Dec 4, 2022
@BDisp
Copy link
Collaborator

BDisp commented Dec 4, 2022

Alright, I think I have everything fixed except the Curses reset issue @BDisp noted.

Still trying to figure that one out...

It seems to be fixed now. Perhaps some Windows 11 update, don't know.

@tig
Copy link
Collaborator Author

tig commented Dec 6, 2022

Curses is still acting weird, but at least I got my VS2022/Windows Terminal config working again. The WT team got me a private build with a fix and I'm now unblocked.

@tig
Copy link
Collaborator Author

tig commented Dec 22, 2022

@BDisp - I cannot get breakpoints to hit when debugging in WSL on two different machines.

I have searched high and low and cannot find out what I'm doing wrong or have mis-configured.

Do you have any suggestions on what I can try?

I can't finish this PR until I fix the bug I caused in the TextView/Editor scenario where copy/paste operations corrupt curses. And I can't figure out what's going on without being able to debug!

Gracias for any help you can provide!

The bug shows itself in many scenarios, not just Editor For example Notepad:

ERRl4Dm 1

@tig
Copy link
Collaborator Author

tig commented Dec 22, 2022

Oh, and a clue to why VS WSL debugging isn't working is this, shown after I end a WSL debug session:

The target process exited without raising a CoreCLR started event. Ensure that the target process is configured to use .NET Core. This may be expected if the target process did not run on .NET Core.
The program '[2440] wsl.exe' has exited with code 0 (0x0).

@BDisp
Copy link
Collaborator

BDisp commented Dec 22, 2022

The easiest way to debug CursesDriver in the Visual Studio 2022 is run with WSL2 selector and attach it to process. Place the breakpoints where you want and voila.

imagem

imagem

@BDisp
Copy link
Collaborator

BDisp commented Dec 22, 2022

@tig this issue is already in the current develop branch. Will another PR caused this or some Windows update?

@tig
Copy link
Collaborator Author

tig commented Jan 2, 2023

@tig this issue is already in the current develop branch. Will another PR caused this or some Windows update?

I see other oddities in develop when using the clipboard in the Editor scenario under WSL, but not screen corruption.

What, specifically, is the issue you see?

(Nevermind, I see you are referring to the Notepad issue:

image

I DO see this in develop.)

(And of course, now that I'm trying to get new video of the Editor bug I think I caused, it no longer repros. Sigh.)

@BDisp
Copy link
Collaborator

BDisp commented Jan 2, 2023

@tig
Copy link
Collaborator Author

tig commented Jan 2, 2023

Based on the fact I can no longer reproduce the Editor issue where curses gets corrupted with this PR, I'm going to merge it. The benefits of all the work I did far outweigh an issue that is so hard to repo on WSL.

If it repos again, we'll just open another issue for it.

@tig tig merged commit 0d183c2 into gui-cs:develop Jan 2, 2023
@tig tig deleted the fixes_2205_fakeclipboard branch April 3, 2024 01:48
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.

Reduce fragility of clipboard related unit tests
3 participants