-
Notifications
You must be signed in to change notification settings - Fork 113
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
test: Add testing of cognee telemetry #573
Conversation
WalkthroughA new GitHub Actions workflow file ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/PR Event
participant WA as GitHub Actions
participant Job as run_common Job
participant Env as Environment Setup
U->>WA: Trigger workflow (manual dispatch/PR event)
WA->>Job: Launch run_common on matrix OS
Job->>Env: Checkout code, setup Python 3.12.x, install Poetry
Env->>Job: Install dependencies
Job->>Job: Create telemetry ID (.anon_id), execute unit tests
Job->>Job: Cleanup temporary files and caches
Job->>WA: Build project package using Poetry
WA-->>U: Report results
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (29)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/test_telemetry.yml (3)
1-2
: Workflow Name Review:
The workflow name “test | test telemetry” is descriptive for its purpose. Consider simplifying the name if it will be displayed in dashboards to improve clarity.
12-15
: Global Environment Variables:
The environment variables are set to enforce an ERROR log level and local environment. Consider if you might later need to parameterize these for different environments.
33-36
: Checkout Action Version:
The workflow usesactions/checkout@master
. It is recommended to pin to a stable version (e.g.,actions/checkout@v3
) to avoid potential disruptions if the default branch changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test_telemetry.yml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (33)
- GitHub Check: Test on macos-15
- GitHub Check: run_notebook_test / test
- GitHub Check: run_multimedia_example_test / test
- GitHub Check: Test on macos-13
- GitHub Check: run_notebook_test / test
- GitHub Check: run_notebook_test / test
- GitHub Check: test
- GitHub Check: run_notebook_test / test
- GitHub Check: Test on macos-15
- GitHub Check: Test on ubuntu-22.04
- GitHub Check: Test on macos-15
- GitHub Check: run_networkx_metrics_test / test
- GitHub Check: Test on macos-15
- GitHub Check: run_simple_example_test / test
- GitHub Check: run_eval_framework_test / test
- GitHub Check: test
- GitHub Check: Test on macos-13
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Test on macos-13
- GitHub Check: run_dynamic_steps_example_test / test
- GitHub Check: windows-latest
- GitHub Check: Test on macos-13
- GitHub Check: Test on ubuntu-22.04
- GitHub Check: Test on ubuntu-22.04
- GitHub Check: Test on ubuntu-22.04
- GitHub Check: lint (ubuntu-latest, 3.11.x)
- GitHub Check: lint (ubuntu-latest, 3.10.x)
- GitHub Check: docker-compose-test
- GitHub Check: Build Cognee Backend Docker App Image
🔇 Additional comments (14)
.github/workflows/test_telemetry.yml (14)
3-7
: Event Trigger Configuration:
The workflow is correctly configured to trigger on both manual dispatch and pull request events (types: labeled, synchronize). This ensures tests run at appropriate times for telemetry changes.
8-11
: Concurrency Settings Validation:
The concurrency group uses a composite key based on the workflow name and either the pull request number or branch reference, which will help prevent redundant runs.
16-32
: Jobs and Matrix Configuration:
Therun_common
job is well configured to run on a matrix of operating systems with clear commentary on the OS labels. The use offail-fast: false
and specifying Bash as the shell ensures consistency across environments.
37-41
: Python Setup Step:
The Python setup step usingactions/setup-python@v5
with version3.12.x
is correctly specified, ensuring the desired Python version is used for the job.
42-49
: Poetry Installation:
The installation of Poetry viasnok/install-poetry@v1.4.1
with in-project virtual environments and parallel installation is effective. Ensure this version aligns with your project’s compatibility requirements.
50-52
: Dependency Installation:
The step invokingpoetry install --no-interaction -E docs -E evals
correctly targets the extra groups. Confirm that the extras “docs” and “evals” include all necessary dependencies for telemetry testing.
53-55
: NLTK Data Download:
The workflow downloads NLTK resources usingpoetry run python -m nltk.downloader punkt_tab averaged_perceptron_tagger_eng
. Please verify thatpunkt_tab
is the intended resource, as the common tokenizer package is usually namedpunkt
.
56-59
: Telemetry Identifier Setup:
The identifier “test-machine” is echoed into.anon_id
, which appears appropriate for telemetry testing. Ensure this file is properly ignored in production builds if not required.
60-62
: Unit Test Execution:
The workflow properly runs unit tests fromcognee/tests/unit/
usingpytest
. Confirm that these tests adequately cover telemetry functionality.
63-65
: Integration Test Execution:
Integration tests are executed oncognee/tests/integration/
, and this step is well configured for telemetry integration testing.
66-77
: Default Basic Pipeline Run:
This step sets several environment variables (including secret references) and runs a basic pipeline test viapython ./cognee/tests/test_library.py
. The use of secrets is appropriate; just ensure they’re masked in logs and only available during test runs.
78-83
: Disk Cleanup Step:
The cleanup step uses sudo privileges to clean cache directories and prints disk usage. Verify that the runner’s permissions allow these commands and that no required caches for debugging or subsequent steps are removed inadvertently.
85-87
: Build with Poetry:
The build step usingpoetry build
is straightforward and correctly placed before package installation.
88-92
: Package Installation:
Changing into thedist
directory and installing the generated wheel viapip install *.whl
is a clear method to test the built package. Consider verifying that only one wheel exists to avoid ambiguity.
.github/workflows/test_telemetry.yml
Outdated
# - ubuntu-22.04: Linux | ||
# - macos-13: macOS Intel | ||
# - macos-15: macOS ARM (public preview) | ||
os: [ubuntu-22.04, macos-13, macos-15] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine if we check it only on ubuntu.
.github/workflows/test_telemetry.yml
Outdated
installer-parallel: true | ||
|
||
- name: Install dependencies | ||
run: poetry install --no-interaction -E docs -E evals |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need docs and evals?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For unit and integration tests I think
Description
Add testing of cognee telemetry
DCO Affirmation
I affirm that all code in every commit of this pull request conforms to the terms of the Topoteretes Developer Certificate of Origin
Summary by CodeRabbit
Tests
Chores