Skip to content

Commit

Permalink
reviewstack: fix verify addons/ folder github ci
Browse files Browse the repository at this point in the history
The container image was missing the gh command line, resulting in CI errors.   Then found it was intemittently failing due to logging after finish, installing the watchman package suppressed the log.

Test plan:

Run the ci.  Before, broken with:
```
Error: Unhandled error. (Error: GhNotInstalledError: Error: Command failed with ENOENT: gh api graphql -f searchQuery=repo:facebook/sapling is:pr author:@me -F numToFetch=100 --hostname github.com -f query=
    query YourPullRequestsQuery($searchQuery: String!, $numToFetch: Int!) {
  search(query: $searchQuery, type: ISSUE, first: $numToFetch) {
    nodes {
      ... on PullRequest {
```

Then found it was intemittently failing due to logging after finish with:
```
  ●  Cannot log after tests are done. Did you forget to wait for something async in your test?
    Attempted to log "Watchman:  Watchman was not found in PATH.  See https://facebook.github.io/watchman/docs/install.html for installation instructions".

      at console.error (../node_modules/@jest/console/build/BufferedConsole.js:163:10)
      at spawnError (node_modules/fb-watchman/index.js:173:13)
      at ChildProcess.<anonymous> (node_modules/fb-watchman/index.js:198:5)
```

Given watch was disabled, added the --no-watchman option as well, plus --detectOpenHandles to show where leaked handles are.

```
Jest has detected the following 3 open handles potentially keeping Jest from exiting:

  ●  Timeout

      814 |   let timedOut = false;
      815 |   if (timeout > 0) {
    > 816 |     const timeoutId = setTimeout(() => {
          |                       ^
      817 |       result.kill('SIGTERM', {forceKillAfterTimeout: 5_000});
      818 |       logger.error(`Timed out waiting for ${command} ${args[0]} to finish`);
      819 |       timedOut = true;

      at src/Repository.ts:816:23
      at src/Repository.ts:14:71
      at Object.<anonymous>.__awaiter (src/Repository.ts:10:12)
      at runCommand (src/Repository.ts:634:12)
      at Repository.runCommand (src/Repository.ts:777:12)
      at Repository.<anonymous> (src/Repository.ts:538:31)
      at src/Repository.ts:14:71
      at Object.<anonymous>.__awaiter (src/Repository.ts:10:12)
      at src/Repository.ts:533:59
      at startAsyncCall (src/utils.ts:47:27)
      at Repository.fetchUncommittedChanges (src/utils.ts:69:14)
      at WatchForChanges.changeCallback (src/Repository.ts:202:14)
      at WatchForChanges.poll (src/WatchForChanges.ts:92:12)
      at new WatchForChanges (src/WatchForChanges.ts:48:10)
      at new Repository (src/Repository.ts:194:28)
      at Object.<anonymous> (src/__tests__/analytics.test.ts:64:18)


  ●  Timeout

      814 |   let timedOut = false;
      815 |   if (timeout > 0) {
    > 816 |     const timeoutId = setTimeout(() => {
          |                       ^
      817 |       result.kill('SIGTERM', {forceKillAfterTimeout: 5_000});
      818 |       logger.error(`Timed out waiting for ${command} ${args[0]} to finish`);
      819 |       timedOut = true;

      at src/Repository.ts:816:23
      at src/Repository.ts:14:71
      at Object.<anonymous>.__awaiter (src/Repository.ts:10:12)
      at runCommand (src/Repository.ts:634:12)
      at Repository.runCommand (src/Repository.ts:777:12)
      at Repository.<anonymous> (src/Repository.ts:610:31)
      at src/Repository.ts:14:71
      at Object.<anonymous>.__awaiter (src/Repository.ts:10:12)
      at src/Repository.ts:601:56
      at startAsyncCall (src/utils.ts:47:27)
      at Repository.fetchSmartlogCommits (src/utils.ts:69:14)
      at WatchForChanges.changeCallback (src/Repository.ts:203:14)
      at WatchForChanges.poll (src/WatchForChanges.ts:92:12)
      at new WatchForChanges (src/WatchForChanges.ts:48:10)
      at new Repository (src/Repository.ts:194:28)
      at Object.<anonymous> (src/__tests__/analytics.test.ts:64:18)


  ●  Timeout

      72 |     }
      73 |
    > 74 |     timeout = setTimeout(callback, wait);
         |               ^
      75 |   }
      76 |
      77 |   debouncer.reset = function () {

      at GitHubCodeReviewProvider.debouncer [as triggerDiffSummariesFetch] (../shared/debounce.ts:74:15)
      at WatchForChanges.changeCallback (src/Repository.ts:206:34)
      at WatchForChanges.poll (src/WatchForChanges.ts:92:12)
      at new WatchForChanges (src/WatchForChanges.ts:48:10)
      at new Repository (src/Repository.ts:194:28)
      at Object.<anonymous> (src/__tests__/analytics.test.ts:64:18)
```

After, works locally, but intermittent problem on CI, shown to be in the timeout handling.
  • Loading branch information
ahornby committed Sep 4, 2023
1 parent fd65dee commit c3206f7
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 3 deletions.
12 changes: 12 additions & 0 deletions .github/workflows/sapling-addons.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ jobs:
container:
image: ${{ format('ghcr.io/{0}/build_ubuntu_20_04:latest', github.repository) }}
steps:
- name: Install gh cli
run: >
apt install -y gpg &&
curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg |
gpg --dearmor -o /usr/share/keyrings/githubcli-archive-keyring.gpg &&
echo "deb [arch=$(dpkg --print-architecture) signed-by=/usr/share/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" |
tee /etc/apt/sources.list.d/github-cli.list > /dev/null &&
apt update && apt install -y gh
- name: Install watchman
run: apt install -y watchman
- name: Checkout Code
uses: actions/checkout@v3
- name: Grant Access
Expand All @@ -23,4 +33,6 @@ jobs:
run: yarn install --prefer-offline
- name: Run addons/verify-addons-folder.py
working-directory: ./addons
env:
GH_TOKEN: ${{ github.token }}
run: ./verify-addons-folder.py
5 changes: 3 additions & 2 deletions addons/isl-server/src/Repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -812,8 +812,9 @@ async function runCommand(
const result = execa(command, args, options);

let timedOut = false;
let timeoutId : NodeJS.Timeout | undefined = undefined;
if (timeout > 0) {
const timeoutId = setTimeout(() => {
timeoutId = setTimeout(() => {
result.kill('SIGTERM', {forceKillAfterTimeout: 5_000});
logger.error(`Timed out waiting for ${command} ${args[0]} to finish`);
timedOut = true;
Expand All @@ -824,7 +825,7 @@ async function runCommand(
}

try {
const val = await result;
const val = await result.finally(() => {clearTimeout(timeoutId);});
return val;
} catch (err: unknown) {
if (isExecaError(err)) {
Expand Down
5 changes: 5 additions & 0 deletions addons/shared/debounce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ export function debounce<Args extends Array<unknown>>(
shouldCallLeading = true;
};

debouncer.dispose = function () {
clearTimeout(timeout);
timeout = undefined;
};

debouncer.isPending = function () {
return timeout != null;
};
Expand Down
2 changes: 1 addition & 1 deletion addons/verify-addons-folder.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ async def verify_reviewstack(*, use_vendored_grammars=False):
async def lint_and_test(cwd: Path):
await asyncio.gather(
run(["yarn", "run", "eslint"], cwd=cwd),
run(["yarn", "test", "--watchAll=false"], cwd=cwd),
run(["yarn", "test", "--no-watch", "--no-watchman", "--detectOpenHandles", "--watchAll=false"], cwd=cwd),
)


Expand Down

0 comments on commit c3206f7

Please sign in to comment.