atom / github

:octocat: Git and GitHub integration for Atom
https://github.atom.io
MIT License
1.12k stars 393 forks source link

Some tests are failing on Linux using (Atom built with) Electron 8 or newer #2688

Open DeeDeeG opened 3 years ago

DeeDeeG commented 3 years ago

Problem

A couple of this repo's tests fail on Linux at the moment. The test failures look like this in CI:

  1) Git commands for CompositeGitStrategy made of [GitShellOutStrategy]
       exec
         when the WorkerManager is not ready or disabled
           kills the git process when cancel is triggered by the prompt server:
     Error: Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/github/github/test/git-strategies.test.js)

  2) Git commands for CompositeGitStrategy made of [GitShellOutStrategy]
       ssh authentication
         falls back to Atom credential prompts if SSH_ASKPASS is present but goes boom:
     Error: Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/github/github/test/git-strategies.test.js)

And this one is flaky as well, not sure if related:

  1) GitPromptServer
       credential helper
         preserves a provided username:
     Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/github/github/test/git-prompt-server.test.js)

Evidence narrowing it down to Electron 8+

I made some Atom builds and re-ran this repo's CI against those, essentially bisecting the Electron releases between Electron 6 (the last version this repo's CI passed on) and Electron 9 (the latest version used in Atom).

Results:

Conclusion: Something must have changed between Electron 7 and 8 that affects a few of this package's tests on Linux.

(Note 1: The last passing CI run at this repo was just before the Electron 9 PR landed at Atom: https://github.com/atom/github/actions/runs/611232541 ran just before https://github.com/atom/atom/pull/21777 was merged. Note 2: This issue on Linux wouldn't be caught at the main Atom repo, because package tests only run on macOS at that repo.)

DeeDeeG commented 3 years ago

I can confirm that I get the same two test failures (and only those two test failures) running this repo's tests on my local machine with atom --test ./test/. So it's not just a CI thing.

Tested with Atom 1.57.0 stable on Ubuntu 20.04.

atom --version ``` $ atom --version Atom : 1.57.0 Electron: 9.4.4 Chrome : 83.0.4103.122 Node : 12.14.1 ```

Edit: If I try to run the tests concurrently with some light web browsing, I can also reproduce the flaky test mentioned above, and another related failure Git commands for CompositeGitStrategy made of [GitShellOutStrategy] --> ssh authentication --> fails the command on authentication failure: --> Error: Timeout of 5000ms exceeded. [. . .]

DeeDeeG commented 3 years ago

These test failures all seem to revolve around credential handling/authentication problems on Linux... at least in CI/tests for this package.

(It's not immediately clear to me if this is a real-world problem or not, but I haven't tried to exercise these features IRL on Linux to see if they work or not.)

If anyone can snoop out the cause of the test failures, that would be great. Or methodically test these theoretical/CI/"test" cases IRL, with the actual github package in actual Atom on Linux, to see if this is an IRL bug or just a bug in the tests.