Skip to content

Conversation

@0xMMBD
Copy link
Contributor

@0xMMBD 0xMMBD commented Dec 10, 2025

Description:

Related issue(s):
#75

Fixes #

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@0xMMBD 0xMMBD force-pushed the feat/75-tests-ci branch 6 times, most recently from cc9883c to 8810c05 Compare December 11, 2025 13:23
@0xMMBD 0xMMBD changed the title wip CI tests feat: add CI workflow for unit, integration and e2e tests Dec 11, 2025
@0xMMBD 0xMMBD marked this pull request as ready for review December 11, 2025 13:24
@0xMMBD 0xMMBD requested review from a team as code owners December 11, 2025 13:24
@0xMMBD 0xMMBD requested a review from rbarker-dev December 11, 2025 13:24

on:
pull_request:
branches: [ main, "feat/75-tests-ci" ]

Choose a reason for hiding this comment

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

Suggested change
branches: [ main, "feat/75-tests-ci" ]
branches:
- main
- feat/75-tests-ci

Prefer to list out the matrix values.

Copy link
Contributor

Choose a reason for hiding this comment

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

And will probably need to remove the feat/75-tests-ci branch before merging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes that branch shouldn't be here, it was here just for testing

uses: ./.github/workflows/run-unit-tests.yml

integration-tests:
needs: [ unit-tests ]

Choose a reason for hiding this comment

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

Prefer to list out the matrix values.

PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }}

e2e-tests:
needs: [ integration-tests ]

Choose a reason for hiding this comment

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

Prefer to list out the matrix values.

OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
ACCOUNT_ID: ${{ secrets.ACCOUNT_ID }}
PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }}
E2E_LLM_PROVIDER: ${{ vars.E2E_LLM_PROVIDER }}

Choose a reason for hiding this comment

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

Suggested change
E2E_LLM_PROVIDER: ${{ vars.E2E_LLM_PROVIDER }}
E2E_LLM_PROVIDER: ${{ inputs.e2e-llm-provider }}

this needs to be passed in as an input

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, I'll adjust that.
It was done in the same way as we have currently in the agent kit JS repo. Do you think it's worth changing it over there too to keep it consistent? @rbarker-dev

description: Working directory
required: false
default: 'python'
type: string

Choose a reason for hiding this comment

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

Suggested change
type: string
type: string
e2e-llm-provider:
description: LLM provider for e2e tests
required: true/false
default: <SOME_VALUE> # please fill this in
type: string


e2e-tests:
needs: [ integration-tests ]
uses: ./.github/workflows/run-e2e-tests.yml

Choose a reason for hiding this comment

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

Suggested change
uses: ./.github/workflows/run-e2e-tests.yml
uses: ./.github/workflows/run-e2e-tests.yml
with:
e2e-llm-provider: <SOME_VALUE>

Comment on lines +6 to +10
workdir:
description: Working directory
required: false
default: 'python'
type: string

Choose a reason for hiding this comment

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

Suggested change
workdir:
description: Working directory
required: false
default: 'python'
type: string
workdir:
description: Working directory
required: false
default: 'python'
type: string
e2e-llm-provider:
description: LLM Provider for E2E Integration Tests
required: true/false
default: <SOME_VALUE>
type: string

OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
ACCOUNT_ID: ${{ secrets.ACCOUNT_ID }}
PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }}
E2E_LLM_PROVIDER: ${{ vars.E2E_LLM_PROVIDER }}

Choose a reason for hiding this comment

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

Suggested change
E2E_LLM_PROVIDER: ${{ vars.E2E_LLM_PROVIDER }}
E2E_LLM_PROVIDER: ${{ inputs.e2e-llm-provider }}

- name: Run integration tests (throttled)
working-directory: ${{ inputs.workdir }}
env:
TEST_DELAY_MS: '8000'

Choose a reason for hiding this comment

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

Suggested change
TEST_DELAY_MS: '8000'
TEST_DELAY_MS: '8000' # <Add a comment for why 8000ms sleep is required>


integration-tests:
needs: [ unit-tests ]
uses: ./.github/workflows/run-integration-tests.yml

Choose a reason for hiding this comment

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

Suggested change
uses: ./.github/workflows/run-integration-tests.yml
uses: ./.github/workflows/run-integration-tests.yml
with:
e2e-llm-provider: <SOME_VALUE>

needs: [ unit-tests ]
uses: ./.github/workflows/run-integration-tests.yml
secrets:
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
LLM_API_KEY: ${{ secrets.OPENAI_API_KEY }}

needs: [ integration-tests ]
uses: ./.github/workflows/run-e2e-tests.yml
secrets:
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
LLM_API_KEY: ${{ secrets.OPENAI_API_KEY }}

default: 'python'
type: string
secrets:
OPENAI_API_KEY:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
OPENAI_API_KEY:
LLM_API_KEY:


on:
pull_request:
branches: [ main, "feat/75-tests-ci" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

And will probably need to remove the feat/75-tests-ci branch before merging.

runs-on: hedera-agent-linux-medium
env:
WORKDIR: python
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
LLM_API_KEY: ${{ secrets.OPENAI_API_KEY }}

default: 'python'
type: string
secrets:
OPENAI_API_KEY:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
OPENAI_API_KEY:
LLM_API_KEY:

runs-on: hedera-agent-linux-medium
env:
WORKDIR: python
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
LLM_API_KEY: ${{ secrets.OPENAI_API_KEY }}

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.

4 participants