-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Run integration tests on Windows #3236
base: master
Are you sure you want to change the base?
Conversation
It is created on Windows when debugging lazygit using VS Code.
They use test/files/pre-push, which doesn't work on Windows.
The "touch" command isn't available on Windows, but "copy NUL file" seems to be a good approximation.
ec02997
to
e2e2b10
Compare
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesYou may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation |
e2e2b10
to
c39066e
Compare
Nice work. Running integration tests on CI is dependent on creack/pty#155 which has been around for ages but may actually be close to merging |
Hi Stefan, I'm seeing a consistent failure on your branch (without the cherry pick from the rev-parse branch) with message:
This is run from cmd shell, with current Go for Windows and Git for Windows. Are you able to see this failure? If not, is there anything I should know about the Windows repro environment? |
Ah, I see. Too bad. Judging from how long this issue has already been going on, I wouldn't be so sure that it's close to merging. I wonder if it's worth switching to photostorm's fork in the meantime, which seems to have the fix already. |
Yeah, sorry I lied when I said they succeed for me. :-/ I thought I had seen a successful run locally, but I must have dreamed that. I'll look into it in the afternoon, we might just have to skip that test too. Unfortunately, using cli to run all tests stops at the first one that fails, so we don't know how many more there are... |
It might be possible to make them work somehow, but for now I'm focusing on getting things green as quickly as possible.
At least they now work when using go run cmd/integration_test/main.go cli or go run cmd/integration_test/main.go tui I haven't been able to get them to run headless (i.e. using "go test pkg/integration/clients/*.go"), which prevents us from running them on CI.
c39066e
to
8311933
Compare
I'm giving up. I disabled a few more tests on Windows in an attempt to make things green; but I can't get a complete run of I also tried moving from creack/pty to photostorm's fork, as mentioned above, hoping that this will make it possible to run the integration tests in "go test" mode. Using photostorm's fork directly wasn't possible because its module declaration is wrong, so I had to make my own. It didn't help though; the code builds now, but running the tests didn't work (seemed to hang without doing anything). My time budget is used up for this. If somebody else feels like picking this up, please do! |
Proposal: let's just disable EVERY remaining Windows test failure from the suite. Get the whole thing, even at reduced capacity, reliably passing via manual runs. That's a lot better than what we have now, and then it becomes possible to work incrementally towards a better future. |
Yes, that's what I tried to do. But I reached a point where tests were either failing or hanging inconsistently, i.e. different ones with every run. Feel free to give it a try too, maybe you have better luck. |
I was hoping to find one of those intermittent failure/hang cases and see if I can catch it in the debugger. That really smells like an integration test harness bug. But it's looking like that's just not going to happen on my Windows ARM VM. The VSCode Go debugger refuses to attach. I already had to swap out go windows arm64 for the Intel build, since one of the VSCode debugging dependencies (dlv) explicitly fails to install as unsupported on ARM. Switching to the Go amd64 build fixes that, but even so the debugger refuses to attach. I'll try again on my Windows Intel VM, but it'll take a bit to set it up for dev work. |
As of this morning, I finally have my Intel Windows VM setup for lazygit work. I can successfully reproduce failures. 😅 I just had a go at attaching a debugger, and it nominally worked, except that the lazygit |
OK, confirmed. The debug workflow in the comment on Ah, this SO answer suggests that the solution is to actually scrape all of the "dlv.exe" processes and check their arguments for a PID matching the waiting program's PID. But it's not clear that PID is actually included in the arguments to dlv, looking at an attached debug session with procmon. :-ppppp |
Relevant Windows API: https://learn.microsoft.com/en-us/windows/win32/api/debugapi/nf-debugapi-isdebuggerpresent Edit: See also this blog post on calling Win APIs from Go |
Fix running integration tests on Windows
At least they now work when using
go run cmd/integration_test/main.go cli
or
go run cmd/integration_test/main.go tui
.I haven't been able to get them to run headless (i.e. using
go test pkg/integration/clients/*.go
), which prevents us from running them on CI.