Skip to content

Conversation

safaricd
Copy link
Contributor

@safaricd safaricd commented Oct 1, 2025

Change Description

Automatically test whenever code runs in a test environment and disable all telemetry.

Known limitations

There are a few limitations we must be aware of with this approach:

  • If _runs_in_test runs before pytest/unittest is imported, the sys.modules check can give a false negative.
  • Code outside of tests may import unittest or even pytest (for tooling or assertions), which would make this return True even though we aren't testing.

Copy link
Contributor

Summary of Changes

Hello @safaricd, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a robust mechanism to automatically detect when the application's code is running within a test environment. The primary purpose is to ensure that telemetry is automatically disabled in such scenarios, preventing test data from polluting production analytics. This change enhances the reliability of telemetry data by isolating it from development and testing activities.

Highlights

  • Telemetry Auto-detection: Implemented a new "_runs_in_test" method within "ProductTelemetry" to automatically detect if the code is executing in a test environment. This method checks for specific environment variables (e.g., PYTEST_CURRENT_TEST), imported testing modules (pytest, unittest, nose), and patterns in sys.argv[0].
  • Telemetry Disablement in Tests: Modified the "telemetry_enabled" method to automatically disable telemetry if the "_runs_in_test" method returns True, ensuring no telemetry data is sent during test runs.
  • Caching for Test Detection: The "_runs_in_test" method is decorated with "@lru_cache(maxsize=1)" to cache its result, optimizing performance by avoiding redundant checks within a single execution context.
  • Unit Tests for Test Detection Logic: Added a new test file "tests/telemetry/core/test_service.py" containing comprehensive unit tests for the "_runs_in_test" method, covering various detection scenarios (environment variables, sys.modules, sys.argv) and verifying its caching behavior.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to automatically detect when the code is running in a test environment to disable telemetry. The implementation in _runs_in_test is comprehensive, checking environment variables, imported modules, and command-line arguments. The accompanying tests are a good start, but have a few issues.

My review focuses on two main points. First, a potential IndexError in _runs_in_test if sys.argv is empty, which could crash the application in certain environments. I've suggested a fix for this. Second, several tests for the sys.argv check are flawed because they don't isolate the logic under test, leading to false positives. I've provided an explanation and a suggested fix for these tests. Addressing these points will make the new feature more robust and the tests more reliable.

safaricd and others added 4 commits October 1, 2025 10:48
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Copy link

@brendan-priorlabs brendan-priorlabs left a comment

Choose a reason for hiding this comment

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

Code itself looks good. Two questions for you,

  1. Were you able to do some manual testing to validate this change? Your callout on module load order in the description was a good one, so I'm curious how well this works in practice.
  2. I thought we already had some exclusions for test environments. Did that get removed earlier somehow?

Returns:
bool: True if telemetry is enabled, False otherwise.
"""
# Overwrite any telemetry if running in tests

Choose a reason for hiding this comment

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

nit: typo, overwrite -> override

@safaricd
Copy link
Contributor Author

safaricd commented Oct 2, 2025

@brendan-priorlabs

Were you able to do some manual testing to validate this change? Your callout on module load order in the description was a good one, so I'm curious how well this works in practice.

Yes, absolutely. It was manually tested inside tabpfn, tabpfn-extensions but also by running tests.

I thought we already had some exclusions for test environments. Did that get removed earlier somehow?

We had, using the TABPFN_DISABLE_TELEMETRY=1 flag, and that is still set in tabpfn/tests. However, other users might run tests and include tabpfn, meaning they most likely wouldn't set the required environmental flag - which was the motivation after all.

Copy link

@brendan-priorlabs brendan-priorlabs left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks!

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