test: Disable failing file-patch tests for Atom CI

[DeeDeeG]

Please be sure to read the contributor's guide to the GitHub package before submitting any pull requests.

Requirements

• Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.

Description of the Change

Disable some failing tests when being run under Atom's Azure Pipelines CI.

Similar situation and solution as was done in #1569 back some time ago.

Screenshot or Gif

N/A

Applicable Issues

Part of a workaround for the mysterious problem in #2607 AKA atom/atom#21782... (ultimately bisected to #2587 but the mechanism of the problem is still unclear.)

Along with atom/atom#21899, this should unblock merging the latest github package into the core Atom repo.

[codecov]

Codecov Report

Merging #2617 (9b5ae9a) into master (8e10ef6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2617   +/-   ##
=======================================
  Coverage   93.45%   93.45%           
=======================================
  Files         237      237           
  Lines       13234    13234           
  Branches     1906     1906           
=======================================
  Hits        12368    12368           
  Misses        866      866           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e10ef6...9b5ae9a. Read the comment docs.

[DeeDeeG]

Hi @smashwilson. I have found a workaround for the bizarre CI failures of github v0.36.5+ in Atom's CI.

Unfortunately, that workaround runs into more test failures. (Slightly less-mysterious ones, though! Perhaps these can be fixed some day soon?) As a somewhat impatient and more-immediate workaround, this PR disables those tests only under Atom's CI.

With this PR and atom/atom#21899, it would enable merging the latest github package to the main Atom repo, and other than the "integration: file-patching" tests (which this present PR disables for Atom CI), all remaining (non-disabled) tests would be passing at the Atom repo's CI. (In other words, unblock atom/atom#21782.)

This is all kind of hard to explain neatly, but nevertheless I wonder if this approach would interest you. It is a bit of a dirty workaround, but (despite its size, when counting atom/atom#21899 over at the main Atom repo) it is the minimal workaround I could find.

Thanks.

- DeeDeeG

P.S. you merged this just before I was going to hit submit on this comment! Thanks for the quick response!

[DeeDeeG]

For the record, this was the output I was seeing for failing github tests over at the main Atom repo's CI, having applied the workaround in atom/atom#21899:

(Just to reiterate and clarify, this present PR disabled the problematic tests for Atom's CI, so we're all good now; Info is presented here in case a proper fix could be found so that this PR's workaround wouldn't be needed anymore.)

  ✓ 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)

Links to these parts of the output in my CI run:

• https://dev.azure.com/DeeDeeG/b/_build/results?buildId=1025&view=logs&j=8a5a6706-d80c-53da-fd15-07e76b4139af&t=c0987c3c-0072-5aba-258a-82ec6b29c5ee&l=2153
• https://dev.azure.com/DeeDeeG/b/_build/results?buildId=1025&view=logs&j=8a5a6706-d80c-53da-fd15-07e76b4139af&t=c0987c3c-0072-5aba-258a-82ec6b29c5ee&l=5401

[DeeDeeG]

This jumps out at me from the above output: Unexpected patch contents

[smashwilson]

Ahhh I vaguely remember this.

I've really thought about just deleting those integration tests outright. I think I've only seen them identify legitimate issues once or twice, but I've seen them cause unrelated problems much, much more. And, we have pretty good coverage of the codebase with the rest of the suite.

I'm 💯 with making a quick fix like this to unblock us!