Skip to content
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

ci(satp-hermes): activate ci for the satp-hermes package #2

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

AndreAugusto11
Copy link
Owner

No description provided.

Copy link
Collaborator

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@AndreAugusto11 Please make sure to update the yarn cache related code to the latest way it's done (it's been updated since the commenting here happened)

An example that's fresh:

  cactus-plugin-keychain-azure-kv:
    continue-on-error: false
    env:
      FULL_BUILD_DISABLED: true
      JEST_TEST_PATTERN: packages/cactus-plugin-keychain-azure-kv/src/test/typescript/(unit|integration|benchmark)/.*/*.test.ts
      JEST_TEST_RUNNER_DISABLED: false
      TAPE_TEST_PATTERN: ./packages/cactus-plugin-keychain-azure-kv/src/test/typescript/integration/plugin-keychain-azure-kv.test.ts
      TAPE_TEST_RUNNER_DISABLED: false
    needs: build-dev
    runs-on: ubuntu-20.04
    steps:
      - name: Use Node.js ${{ env.NODEJS_VERSION }}
        uses: actions/setup-node@v3.6.0
        with:
          node-version: ${{ env.NODEJS_VERSION }}
      - uses: actions/checkout@v3.5.2

      - id: yarn-cache
        name: Restore Yarn Cache
        uses: actions/cache@v3.3.1
        with:
          key: ${{ runner.os }}-yarn-${{ hashFiles('./yarn.lock') }}
          path: ./.yarn/
          restore-keys: |
            ${{ runner.os }}-yarn-${{ hashFiles('./yarn.lock') }}
      - run: ./tools/ci.sh

Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>
@AndreAugusto11
Copy link
Owner Author

@petermetz thanks, I've just updated!

@AndreAugusto11
Copy link
Owner Author

AndreAugusto11 commented Mar 8, 2024

@petermetz the ci hang up when running the satp tests because they are not finishing. I was able to reproduce this on my machine and this is the result:

Screenshot 2024-03-08 at 09 46 15

I have been trying to understand what's going on (like the connection to the databases), but it seems that the error persists even if there is nothing but the test ledgers and connectors involved -- i.e., nothing related to the SATP package.

I've tried using the flags --runInBand --detectOpenHandles, but when I do, the test passes and closes gracefully...

I have tried running, for example, a test from within the Fabric connector that creates a test ledger, and it goes perfectly. The difference is that those are using TAP and not JEST. Could it be something related to JEST itself?

I've put together a test called experiment-open-handles.test.ts to reproduce this. On my local machine I get the sma error (even though it actually disconnects a second later as seen below).

Screenshot 2024-03-08 at 09 53 52

Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>
Signed-off-by: André Augusto <andre.augusto@tecnico.ulisboa.pt>
@AndreAugusto11
Copy link
Owner Author

@petermetz I'm having the same issue with this test file packages/cactus-plugin-ledger-connector-besu/src/test/typescript/integration/plugin-ledger-connector-besu/deploy-contract/invoke-contract.test.ts

Screenshot 2024-03-08 at 11 03 18

@AndreAugusto11
Copy link
Owner Author

It seems like all tests involving ledgers are having this issue, but they exit immediately after the warning is thrown. There are two tests in the satp package that are throwing the warning but hang the terminal. These must have something else going on, that I'm going to explore.

@petermetz
Copy link
Collaborator

It seems like all tests involving ledgers are having this issue, but they exit immediately after the warning is thrown. There are two tests in the satp package that are throwing the warning but hang the terminal. These must have something else going on, that I'm going to explore.

@AndreAugusto11 I'm aware of two issues that could be affecting this. One is the Fabric AIO image troubles which I just very recently addressed on the main branch (last week or so) - I sent you a commit in a GH comment on another pull request discussing the same (I'm not sure now if this was a different PR from the one I'm commenting on right now, sorry, too many pull requests to keep straight)

The other issue is that some tests don't clean up after themselves correctly after a failure and leave HTTP servers running and you hard-code the ports in your tests (it is advised not to) so what happens (happened to me when I was testing the SATP tests) is that one test fails, but it leaves the port 3000 for example occupied. Then from then on even if you run another test that is normally passing and all good, it will still break immediately because the port is taken and it just taps out.
The same thing can happen with ledger ports if your test left the Fabric AIO image running (for that one we have no choice but to hard code the ports it listens on unfortunately)

So I'd check these two things and also make sure to look at the commit I linked to in my other comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants