Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Conversation

@smashwilson
Copy link
Contributor

Upgrade the GitHub package on the nightly channel to the latest release.

@smashwilson
Copy link
Contributor Author

It looks like the new superstring version is breaking:

..\src\core\encoding-conversion.cc(3): fatal error C1083: Cannot open include file: 'iconv.h': No such file or directory [D:\a\1\s\node_modules\whats-my-line\node_modules\superstring\build\superstring_core.vcxproj]

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Dec 5, 2020

I was able to make a narrower steps to reproduce for this error message.

Maybe this submodule needs to be initialized during a preinstall script for the superstring package? (If that's true, it's unclear why this would have never come up before now.)

Edit: or switch to a more Node-ecosystem-friendly solution like the iconv package or iconv-lite?

@aminya
Copy link
Contributor

aminya commented Dec 6, 2020

It looks like the new superstring version is breaking:

..\src\core\encoding-conversion.cc(3): fatal error C1083: Cannot open include file: 'iconv.h': No such file or directory [D:\a\1\s\node_modules\whats-my-line\node_modules\superstring\build\superstring_core.vcxproj]

iconv submodule should be initiated before publishing to npm.

I made a PR: atom/superstring#91

@smashwilson
Copy link
Contributor Author

@sadick254 I'm guessing we'll need to cut new releases of superstring, whats-my-line, and atom/github to get this passing 😬

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Dec 7, 2020

whats-my-line doesn't strictly need to be updated, but the other two packages would need updates.

(whats-my-line uses an "at least this version" range for superstring. That version range would accept any patch or minor bump of superstring.)

So, this would do it:

  • cut a new release of superstring pushed to the npm registry with win-iconv included
    • (Thanks to @aminya's PR, win-iconv should now be synced automatically during npm publish, no need to manually initialize the submodule on CLI anymore.)
  • bump superstring in github's package-lock.json
    • cut a new release of github including this dependency bump
    • (because Atom's script/build recompiles github with all of its exact package-lock.json dependencies, I think?)

@sadick254
Copy link
Contributor

@smashwilson Yes we do. I will be doing the releases in a short while.

@smashwilson
Copy link
Contributor Author

Now it looks like the GitHub package tests are timing out the build 😓

##[command] Executing github tests

##[warning] Retrying the timed out step: github package 


##[warning] Retrying the timed out step: github package 

It looks like the macOS builds there are running in about 14 minutes, although of course that's on a GitHub Actions runner instead of Azure DevOps so it might not be completely equivalent.

I'm wondering if we should split the GitHub package out completely into its own matrix row there. None of the other bundled packages have nearly that long of a test time, so I feel like it's going to overwhelm whatever row it's on. I might also be able to add some internal logic to skip lengthy integration tests and such as part of a full Atom build.

@aminya
Copy link
Contributor

aminya commented Dec 8, 2020

The separation of the GitHub tests at the CI level has no benefits. They are already taken care of the test runner (they run in separate child processes).

The retrying is because of the timeout trigger I added to the tests (I defined this error as timing out). Disable the following line, and it will not retry them anymore:

'Error Downloading Update: Could not get code signature for running application'

@smashwilson
Copy link
Contributor Author

@aminya Ah! Thanks for the context. I pretty clearly haven't been paying attention here for a while 😅

I'll give it a shot with that line removed from the conditional. I'd thought that error wasn't fatal, but I'm pretty rusty so I could be mistaken 🤔

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Dec 8, 2020

For more context, there were some PRs earlier this year to update script/test:

@aminya
Copy link
Contributor

aminya commented Dec 16, 2020

These tests were flaky before too, that's why I had added that time-out trigger.

My instinct says that the offending part is using MacOS 1.15 which is caused by this PR:
0a169b5

Has MacOS changed anything regarding code signing, app signature, etc?

@smashwilson Could you revert this and this, and run the tests again.

@aminya aminya mentioned this pull request Dec 16, 2020
58 tasks
@smashwilson
Copy link
Contributor Author

These tests were flaky before too, that's why I had added that time-out trigger.

Yah, I remember seeing these Decorations test hard-crash Electron occasionally for a long time, now (although, it never crossed the threshold to be worth really trying to get to the bottom of it). But on this branch I've never seen a build that didn't hit it, which leads me to believe that in this case the failure is legitimate. I'm wondering if these are made more likely by the superstring changes (the only related native code changes I'm aware of) or Electron changes (has there been an Electron upgrade on atom/atom master since the last time I released?).

The "Error Downloading Update" message is, as far as I know, entirely unrelated. I believe it's written to stderr even when the tests are passing, you just don't see it because we only dump stderr for failing suites.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Dec 17, 2020

I have been doing a ton of CI runs to bisect this issue, feel free to take a look as I narrow it down.

https://dev.azure.com/DeeDeeG/b/_build?definitionId=17&_a=summary

Branch: https://github.com/DeeDeeG/atom/commits/superstring-up with commits+CI status icons (which can be clicked through to see the respective runs)

Tentative lead: Something changed in github 0.36.5???? Patching 0.36.4 to have newer superstring does not cause this CI issue.

@smashwilson
Copy link
Contributor Author

@DeeDeeG thank you for helping out with this 😍

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Dec 17, 2020

This PR atom/github#2587 is the only code change I'm seeing.

I will put together a CI run with that PR reverted. I know it's an important bug fix, but it might be tripping up Atom's CI for reasons as yet unknown.

(No code changed in 0.36.5 other than the GraphQL schema update, which I think was very small in 0.36.5). Other than that, we have some very trivial dependency changes (whats-my-line, which in turn only bumped its minimum superstring (going from an already-generous ^2.4.2 semver range ("at least 2.4.2 in the 2.x major version") to ^2.4.3 -- this change didn't strictly need to happen IMO.)). Nothing weird got published in the tarballs for the whats-my-line package so I think that's clear/okay. The other PR strictly updates markdown documentation files. So by process of elimination, it looks like atom/github#2587, which touched both the libraries and the tests. Probably that, then. 🤔)

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Dec 17, 2020

@smashwilson reverting that PR (atom/github#2587) allows the macOS "package tests 1" job to finish.

Example passing Atom CI: https://dev.azure.com/DeeDeeG/b/_build/results?buildId=920&view=results

@smashwilson
Copy link
Contributor Author

Bizarre... Those tests are all green on atom/github's CI.

I suppose the next step would be to try to reproduce the failures in atom/github with a local dev build of Atom's master?

It's strange that it's a crash instead of a test failure, too.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Dec 17, 2020

Not sure if this helps, but I got this when running the Atom "package tests 1" set on Windows: https://gist.github.com/DeeDeeG/91429219c191794c5df52d8797bc2f61

On macOS, for me at some point during the github tests the copy of Atom running the tests exited, and script/test simply stopped running (my terminal prompt was ready to run another command). Could just be that my macOS computer is slow...

@aminya
Copy link
Contributor

aminya commented Dec 17, 2020

@smashwilson Please try what I suggested in #21782 (comment).

In the GitHub repo, you are running the tests in MacOS Mojave, but this repo switched to Catalina recently.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Dec 17, 2020

@aminya you may be onto something... The "Nightly" pipeline hasn't had a successful build since November 8.

https://github.visualstudio.com/Atom/_build?definitionId=1&_a=summary

Among other things that have changed since then, the build would have been done on macOS 10.14 Mojave back then.

(In any case, I am running the github package's CI against one of my recent Atom Dev builds, will see what happens: https://github.com/DeeDeeG/github/runs/1572963037)

Two separate questions come to my mind: Does building Atom on macOS 10.15 (Catalina) reproduce this CI failure? Does running the tests on macOS 10.15 reproduce the CI failure? Only the combination of both? Will try to run enough tests to sort it out.


Related (CI failures affecting not just Nightly but Production/master pipelines)

I believe CI should use the GitHub file download API, and do so with authentication. Either it's trying to download "certificates" that don't exist, or GitHub is only telling CI the file doesn't exist because it's trying to download them unauthenticated.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Dec 17, 2020

@aminya
Copy link
Contributor

aminya commented Jan 1, 2021

I found the reasons for the failing tests!

GitHub's CI needs to be updated as I mentioned in:
atom/github#2459 (comment)

Here are the details of the errors:
https://github.com/UziTech/github/pull/9/checks?check_run_id=1633567905#step:6:5221

@smashwilson
Copy link
Contributor Author

@aminya those look like different errors, though?

The integration tests that are failing there have always been kind of brittle - they rely on receiving actual filesystem events and the DOM updating in a different manner. I've debated taking them out altogether because they seem to cause much more build trouble than they identify legitimate problems. The failures that we're seeing in the Atom build here are hard Electron crashes in the Decoration suite for some reason.

@smashwilson
Copy link
Contributor Author

When I get time I'll have to split the crashes in here into their own atom/github issue 🤔

@aminya
Copy link
Contributor

aminya commented Jan 1, 2021

@aminya those look like different errors, though?

The integration tests that are failing there have always been kind of brittle - they rely on receiving actual filesystem events and the DOM updating in a different manner. I've debated taking them out altogether because they seem to cause much more build trouble than they identify legitimate problems. The failures that we're seeing in the Atom build here are hard Electron crashes in the Decoration suite for some reason.

setup-atom downloads and sets up the official atom releases from different channels, and so there is no difference in terms of the environment. If the tests fail, it should be related to the code itself.

Maybe we should first get this PR working before trying to fix the Electron failures?

@smashwilson
Copy link
Contributor Author

Do you mean get atom/github#2459 working and merged? That's fine, but we'll still need to fix the Electron crashes before we can merge this PR, because that's the only failure that's appearing here.

@aminya
Copy link
Contributor

aminya commented Jan 1, 2021

No, I literally meant this PR 😄. #21782 and #21783. I think the Electron PR will take some time until it gets ready ready.

The bugs fixed in these two PRs (that go to beta and dev) are important, IMO.

@smashwilson
Copy link
Contributor Author

Maybe we should first get this PR working before trying to fix the Electron failures?

I'm confused about this, then.

The reason this PR is not mergeable is because of the native crashes. What do you mean by "get this PR working" that isn't fixing those?

@aminya
Copy link
Contributor

aminya commented Jan 1, 2021

@smashwilson Never mind 😄 Definitely, you have more context about the issue. I thought that the test failures in #21782 and #21783 are different from the errors that happen in the Electron PR.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 6, 2021

It's strange that it's a crash instead of a test failure, too.

"Crashing" may be the usual quitting behavior for Atom on macOS when not code-signed. In practice, nothing obviously bad is happening in the GUI. I always see a crash event mentioned when closing a window, if running non-code-signed Atom via the command-line with full path to the binary, like this: out/Atom\ Dev.app/Contents/MacOS/Atom

% out/Atom\ Dev.app/Contents/MacOS/Atom\ Dev 
Error Downloading Update: Could not get code signature for running application
Attempting to call a function in a renderer window that has been closed or released.
Function provided here: worker.js:79:22
Remote event names: crashed, destroyed

Perhaps (reading possibly way too far into the scant logging info) there is a worker responsible for getting updates, and perhaps that worker doesn't exit if it can't read a code signature. Then it's forced to exit less-than-gracefully the next time a window closes.

Like I mentioned above, this is not a noticeable problem for end-users that I know of. (Exiting cleanly would be good, but speaking for myself, I think there may be bigger things to worry about in Atom development than exiting this worker cleanly? In which case this could be seen rather as a CI issue than a production issue. We could try to ignore this particular error message, perhaps re-logging it as info rather than treating it as a full-on CI-failing level of error.)

@smashwilson
Copy link
Contributor Author

Crashes filed as atom/github#2607.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 16, 2021

My above comment may be off-base -- I don't think this resembles the usual background noise of "closing a window prints a message about a crash event." With this bug, I see the test output trying to print what test was running, and the test output is cut off in the middle of the line. So I think the crash bubbles all the way up to the test runner itself? Or maybe just the test runner is crashing?

(As @smashwilson mentioned here: #21782 (comment), this bug/crash/phenomenon is reliably happening in the github package's decorations test suite.)

Here's an example run, linking to the line where the text is cut off part-way through: https://dev.azure.com/DeeDeeG/b/_build/results?buildId=979&view=logs&j=6de74252-975b-522d-7a3d-227ca79ca6c1&t=a70a2b1c-2767-57d7-6509-edd8e9181730&l=145

The text cut off part-way through is: Decoration with a and it comes right after Decoration does not attempt to decorate a destroyed TextEditor.

@smashwilson @aminya I think I've narrowed it down to this test group: https://github.com/atom/github/blob/v0.36.6/test/atom/decoration.test.js#L42-L167 (Decoration with a subtree)

(This is the only part of the decoration suite that has the text with a, which is the text I'm seeing being cut off in the middle of printing the test name).


P.S. These tests are running in parallel if I understand correctly, so I suppose the issue could be in one of the other nearby tests, not necessarily the one which has its name truncated.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 16, 2021

I tried this with parallelization in script/test reverted, and the output is looking a bit different.

Baseline

As a baseline or control to compare against, here is the code and dependencies of the current master branch tested with the old test runner script:

  • Old non-parallelized script/test and old files in script/vsts, but otherwise identical to master branch:

Result: "macOS test packages-1" job is passing

Same thing with updated github package

Now the more interesting stuff:

  • Same scenario, but with github bumped to 0.36.6:

Result: We get a strange line of output at the end:

    PaneItem when opened with a matching URI renders a differError! The 'package-github' test step finished with a non-zero exit code

Note that this line begins to print out the name of the test, but part way through the test name is interruted with an Error! message.


Takeaway

This would seem to narrow it down to this entirely different test than what I concluded in my previous comment:
https://github.com/atom/github/blob/v0.36.6/test/atom/pane-item.test.js#L150-L162

I don't exactly know how to interpret these results. So take with a grain of salt (and possibly scrutinize the methodology here for any context clues it can give?). I also find it very strange that the names of the tests keep getting cut off mid-way. That goes a bit beyond the usual error presentation in these tests, I think.


P.S. second run gives the same test group from the previous comment: Decoration with a subtree. CI link

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 16, 2021

Actually, I'm mostly seeing the same Decoration with a subtree tests failing, upon re-running a couple of times.

@aminya
Copy link
Contributor

aminya commented Jan 16, 2021

I tried this with parallelization in script/test reverted, and the output is looking a bit different.

There is no parallelization in the tests. If you want to run them in parallel you should change async.series to async.parallel

async.series(testSuitesToRun, function(err, results) {

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 17, 2021

Okay, thanks. I must have misremembered -- I think that greater test parallelization was attempted to be added at some point, but then it got changed to series. Or something like that.

I didn't learn a lot from my experiment in that comment... It turns out the result is the same, and still just as mysterious.

I am trying everything I can think of to narrow down the cause of this strange issue, or at least get CI to start passing here.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 21, 2021

I have found that the github package tests can pass when run in Atom's CI without being run under script/test.

Just download the built Atom in ./out/ into a CI job, cd into and apm install github's dependencies (loads the devDependencies/ test runner dependencies), and run the Atom --test command manually:
./out/Atom\ Dev.app/Contents/MacOS/Atom\ Dev --resource-path $(CI_workdirpath) --test node_modules/github/test

I think it's another one of those "weird abrupt quitting behavior when run as a child_process" bugs. Last time we saw that a Node upgrade fixed it, but the latest Node 12 and Node 14 don't seem to help with this.

The downside of running outside of script/test for now is that I don't see an obvious way to repeat for timeouts. Maybe just try/catch and rerun up to n times if it exits with status other than 0.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 22, 2021

This error reproduced at least once with github 0.36.3!

https://github.visualstudio.com/Atom/_build/results?buildId=96316&view=logs&j=6de74252-975b-522d-7a3d-227ca79ca6c1&s=d654deb9-056d-50a2-1717-90c08683d50a&t=a70a2b1c-2767-57d7-6509-edd8e9181730&l=999

Perhaps this new reproduced case can help zero in on the true cause.

Test output snippet (click to expand):
  ✓ Decoration does not attempt to decorate a destroyed Marker: 1ms
    Decoration does not attempt to decorate a destroyed TextEditor: 
  ✓ Decoration does not attempt to decorate a destroyed TextEditor: 2ms
    Decoration with a subtree creates a block decoration: 
  ✓ Decoration with
 
 ##[error] *** Reporting the errors that happened in all of the tests: *** 
 

##[error] The 'find-and-replace package' test step finished with a non-zero exit code 
 You can run the test again using: 
	 /Users/runner/work/1/s/out/Atom Dev.app/Contents/MacOS/Atom Dev --resource-path /Users/runner/work/1/s --test /Users/runner/work/1/s/node_modules/find-and-replace/spec
##[error] The 'github package' test step finished with a non-zero exit code 
 You can run the test again using: 
	 /Users/runner/work/1/s/out/Atom Dev.app/Contents/MacOS/Atom Dev --resource-path /Users/runner/work/1/s --test /Users/runner/work/1/s/node_modules/github/test

##[error]PowerShell exited with code '1'.

CI run based on commit 0008c25 (not saying that commit is related, or not related, to this reproducing. I'm not sure. Just documenting this info in case the CI run gets deleted by Azure Pipelines.)

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jan 23, 2021

I ran the tests in bash rather than Powershell to see if anything would change. The decorations tests finished, but the text was cut off for a significantly later test: https://dev.azure.com/DeeDeeG/b/_build/results?buildId=1029&view=logs&j=6de74252-975b-522d-7a3d-227ca79ca6c1&t=56617b1b-c70c-5161-3a8b-aa8f5dbad8f2&l=253 (something to do with pane items). This is the same later test I saw getting interrupted, when running the tests on the old version of script/test.

Interestingly, when running outside of script/test entirely, I see slightly more mundane timeouts in the integration tests that need to do disk access:

  ✓ integration: check out a pull request opens a pane item for a pull request by clicking on an entry in the GitHub tab: 1062ms
    integration: file patches with an added file unstaged may be partially staged: Unexpected patch contents:
   0000 cursor-line github-FilePatchView-line--selected github-FilePatchView-line--added x"github-FilePatchView-line--selected"
  0001 github-FilePatchView-line--added
  0002
  0003
  0004 github-FilePatchView-line--added -"github-FilePatchView-line--selected"
  0005 github-FilePatchView-line--added

    integration: file patches with an added file unstaged may be partially staged: 
  ✓ integration: file patches with an added file unstaged may be partially staged: 449ms
    integration: file patches with an added file unstaged may be completed staged: 
  ✓ integration: file patches with an added file unstaged may be completed staged: 310ms
    integration: file patches with an added file unstaged may discard lines: 
  ✓ integration: file patches with an added file unstaged may discard lines: 313ms
    integration: file patches with an added file staged may be partially unstaged: 
  1) integration: file patches with an added file "before each" hook for "may be partially unstaged"
    integration: file patches with a removed file unstaged may be partially staged: 
  2) integration: file patches with a removed file unstaged "before each" hook for "may be partially staged"
    integration: file patches with a removed file staged may be partially unstaged: 
  ✓ integration: file patches with a removed file staged may be partially unstaged: 469ms
    integration: file patches with a removed file staged may be completely unstaged: 
  ✓ integration: file patches with a removed file staged may be completely unstaged: 371ms
    integration: file patches with a symlink that used to be a file unstaged may stage the content deletion without the symlink creation: 
  ✓ integration: file patches with a symlink that used to be a file unstaged may stage the content deletion without the symlink creation: 413ms
    integration: file patches with a symlink that used to be a file unstaged may stage the content deletion and the symlink creation: 
  3) integration: file patches with a symlink that used to be a file unstaged "before each" hook for "may stage the content deletion and the symlink creation"
  - integration: file patches with a symlink that used to be a file staged may unstage the symlink creation but not the content deletion
  - integration: file patches with a file that used to be a symlink unstaged may stage the symlink deletion without the content addition
  - integration: file patches with a file that used to be a symlink unstaged may stage the content addition and the symlink deletion together
    integration: file patches with a file that used to be a symlink staged may unstage the content addition and the symlink deletion together: 
  4) integration: file patches with a file that used to be a symlink staged "before each" hook for "may unstage the content addition and the symlink deletion together"
    integration: file patches with a modified file unstaged may be partially staged: 
  ✓ integration: file patches with a modified file unstaged may be partially staged: 540ms
    integration: file patches with a modified file unstaged may be completely staged: 
  5) integration: file patches with a modified file unstaged "before each" hook for "may be completely staged"

[ . . . ]
[ . . . ]
[ . . . ]

  2458 passing (9m)
  4 pending
  5 failing

  1) integration: file patches
       with an added file
         "before each" hook for "may be partially unstaged":
     Error: async(9000ms): timed out waiting until the list item for path added-file.txt (unstaged) appears
      at /Users/runner/work/1/s/node_modules/github/node_modules/test-until/index.js:56:25

  2) integration: file patches
       with a removed file
         unstaged
           "before each" hook for "may be partially staged":
     Error: async(9000ms): timed out waiting until the list item for path sample.js (unstaged) appears
      at /Users/runner/work/1/s/node_modules/github/node_modules/test-until/index.js:56:25

  3) integration: file patches
       with a symlink that used to be a file
         unstaged
           "before each" hook for "may stage the content deletion and the symlink creation":
     Error: async(9000ms): timed out waiting until the list item for path sample.js (unstaged) appears
      at /Users/runner/work/1/s/node_modules/github/node_modules/test-until/index.js:56:25

  4) integration: file patches
       with a file that used to be a symlink
         staged
           "before each" hook for "may unstage the content addition and the symlink deletion together":
     Error: async(9000ms): timed out waiting until the list item for path symlink.txt (staged) appears
      at /Users/runner/work/1/s/node_modules/github/node_modules/test-until/index.js:56:25

  5) integration: file patches
       with a modified file
         unstaged
           "before each" hook for "may be completely staged":
     Error: async(9000ms): timed out waiting until the list item for path sample.js (unstaged) appears
      at /Users/runner/work/1/s/node_modules/github/node_modules/test-until/index.js:56:25

CI link with 0.36.6

When I patch github to have longer timeouts (60 seconds!), I get a similar version of the error:

  ✓ integration: check out a pull request opens a pane item for a pull request by clicking on an entry in the GitHub tab: 1090ms
    integration: file patches with an added file unstaged may be partially staged: Unexpected patch contents:
   0000 cursor-line github-FilePatchView-line--selected github-FilePatchView-line--added x"github-FilePatchView-line--selected"
  0001 github-FilePatchView-line--added
  0004 github-FilePatchView-line--added -"github-FilePatchView-line--selected"
  0005 github-FilePatchView-line--added

    integration: file patches with an added file unstaged may be partially staged: Unexpected patch contents:
   0000 cursor-line github-FilePatchView-line--selected github-FilePatchView-line--added x"github-FilePatchView-line--selected"
  0001 github-FilePatchView-line--added
  0002
  0003
  0004 github-FilePatchView-line--added -"github-FilePatchView-line--selected"
  0005 github-FilePatchView-line--added

    integration: file patches with an added file unstaged may be partially staged: Unexpected patch contents:
   0000 cursor-line github-FilePatchView-line--selected github-FilePatchView-line--added x"github-FilePatchView-line--selected"
  0001 github-FilePatchView-line--added
  0002
  0003
  0004 github-FilePatchView-line--added -"github-FilePatchView-line--selected"
  0005 github-FilePatchView-line--added

    integration: file patches with an added file unstaged may be partially staged: 
  ✓ integration: file patches with an added file unstaged may be partially staged: 469ms
    integration: file patches with an added file unstaged may be completed staged: 
  1) integration: file patches with an added file "before each" hook for "may be completed staged"
    integration: file patches with a removed file unstaged may be partially staged: 
  ✓ integration: file patches with a removed file unstaged may be partially staged: 562ms
    integration: file patches with a removed file unstaged may be completely staged: 
  2) integration: file patches with a removed file unstaged "before each" hook for "may be completely staged"
    integration: file patches with a removed file staged may be partially unstaged: 
  ✓ integration: file patches with a removed file staged may be partially unstaged: 495ms
    integration: file patches with a removed file staged may be completely unstaged: 
  3) integration: file patches with a removed file staged "before each" hook for "may be completely unstaged"
    integration: file patches with a symlink that used to be a file unstaged may stage the content deletion without the symlink creation: 
  4) integration: file patches with a symlink that used to be a file unstaged "before each" hook for "may stage the content deletion without the symlink creation"
  - integration: file patches with a symlink that used to be a file staged may unstage the symlink creation but not the content deletion
  - integration: file patches with a file that used to be a symlink unstaged may stage the symlink deletion without the content addition
  - integration: file patches with a file that used to be a symlink unstaged may stage the content addition and the symlink deletion together
    integration: file patches with a file that used to be a symlink staged may unstage the content addition and the symlink deletion together: 
  ✓ integration: file patches with a file that used to be a symlink staged may unstage the content addition and the symlink deletion together: 488ms
    integration: file patches with a file that used to be a symlink staged may unstage the content addition without the symlink deletion: 
  ✓ integration: file patches with a file that used to be a symlink staged may unstage the content addition without the symlink deletion: 765ms
    integration: file patches with a modified file unstaged may be partially staged: 
  ✓ integration: file patches with a modified file unstaged may be partially staged: 805ms
    integration: file patches with a modified file unstaged may be completely staged: 
  5) integration: file patches with a modified file unstaged "before each" hook for "may be completely staged"
    integration: file patches with a modified file staged may be partially unstaged: 
  6) integration: file patches with a modified file staged "before each" hook for "may be partially unstaged"

[ . . . ]
[ . . . ]
[ . . . ]

  2455 passing (18m)
  4 pending
  6 failing

  1) integration: file patches
       with an added file
         "before each" hook for "may be completed staged":
     Error: Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/runner/work/1/s/node_modules/github/test/integration/file-patch.test.js)
  

  2) integration: file patches
       with a removed file
         unstaged
           "before each" hook for "may be completely staged":
     Error: Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/runner/work/1/s/node_modules/github/test/integration/file-patch.test.js)
  

  3) integration: file patches
       with a removed file
         staged
           "before each" hook for "may be completely unstaged":
     Error: Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/runner/work/1/s/node_modules/github/test/integration/file-patch.test.js)
  

  4) integration: file patches
       with a symlink that used to be a file
         unstaged
           "before each" hook for "may stage the content deletion without the symlink creation":
     Error: Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/runner/work/1/s/node_modules/github/test/integration/file-patch.test.js)
  

  5) integration: file patches
       with a modified file
         unstaged
           "before each" hook for "may be completely staged":
     Error: async(60000ms): timed out waiting until the list item for path sample.js (unstaged) appears
      at /Users/runner/work/1/s/node_modules/github/node_modules/test-until/index.js:56:25

  6) integration: file patches
       with a modified file
         staged
           "before each" hook for "may be partially unstaged":
     Error: Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/runner/work/1/s/node_modules/github/test/integration/file-patch.test.js)

CI link with 0.36.6 patched to have 60 second timeouts

These timeouts do not happen for me under the otherwise same testing scenario, but with github 0.36.3)


There is an open issue for exactly one out of this group of test failures at the github repo... atom/github#2149 I read and it exactly matches what I'm seeing (when running github's tests manually outside of script/test). I assume they all share a root cause though, even though I'm seeing more (but adjacent) failures than he reported in that issue.

Edit: Another issue for these flaky filesystem-accessing tests: atom/github#1769

DeeDeeG added a commit to DeeDeeG/atom that referenced this pull request Jan 27, 2021
This job runs outside of `script/test`, without using child_processes.

It appears to help github's tests pass as of the v0.36.5 release.

See atom#21782
for discussion of (and attempts to debug) this issue.
Copy link
Contributor

@aminya aminya left a comment

Choose a reason for hiding this comment

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

This should be closed now after merging #21903

@smashwilson
Copy link
Contributor Author

Yup 👍🏻

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants