Skip to content

Don't run if CI isn't set#71

Merged
meghan-kradolfer merged 2 commits intobuildkite:mainfrom
jmsanders:jordan/no-CI
Jul 29, 2025
Merged

Don't run if CI isn't set#71
meghan-kradolfer merged 2 commits intobuildkite:mainfrom
jmsanders:jordan/no-CI

Conversation

@jmsanders
Copy link
Contributor

This should quiet these warnings when running pytest locally on developer machines outside of CI:

buildkite-test-collector - WARNING - No BUILDKITE_ANALYTICS_TOKEN environment variable present

Based on the README, I think this was already the intent:

Run your tests like normal. Note that we attempt to detect the presence of several common CI environments, however if this fails you can set the CI environment variable to any value and it will work.

This should quiet these warnings when running pytest locally on
developer machines outside of CI:

```
buildkite-test-collector - WARNING - No BUILDKITE_ANALYTICS_TOKEN environment variable present
```

Based on the README, I think this was already the intent:

> Run your tests like normal. Note that we attempt to detect the presence of several common CI environments, however if this fails you can set the `CI` environment variable to any value and it will work.
Copy link
Contributor

@nprizal nprizal left a comment

Choose a reason for hiding this comment

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

Hi @jmsanders, thanks for opening the PR.

We’ve intentionally removed the CI env requirements (see #46) because we want the collector to run outside of CI for debugging purpose. Your changes only skip the upload process when the CI env is missing, which is a valid case. I'm happy with the changes, but could you please rename the variable to ENV_CI for clarity?

class API:
"""Buildkite Test Engine API client"""

CI_TOKEN = "CI"
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
CI_TOKEN = "CI"
ENV_CI = "CI"


def __init__(self, env: Mapping[str, Optional[str]]):
"""Initialize the API client with environment variables"""
self.ci = env.get(self.CI_TOKEN)
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
self.ci = env.get(self.CI_TOKEN)
self.ci = env.get(self.ENV_CI)

@jmsanders jmsanders requested a review from nprizal July 29, 2025 13:48
@jmsanders
Copy link
Contributor Author

Sure thing - changed.

@meghan-kradolfer meghan-kradolfer merged commit c5ebd35 into buildkite:main Jul 29, 2025
10 checks passed
@meghan-kradolfer meghan-kradolfer mentioned this pull request Jul 29, 2025
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.

3 participants