refactor: Remove need for RUN_PLATFORM_TESTS env var, rely on TESTER_APIFY_TOKEN - #122
Conversation
…ORM_TESTS Platform test suites now run whenever a tester token is present, so there is no separate opt-in flag to remember. RUN_ALL_PLATFORM_TESTS keeps working as before. Closes #121 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JSd2mJ6ZFg8cf7dRg4nTQg
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JSd2mJ6ZFg8cf7dRg4nTQg
metalwarrior665
changed the title
ruocco-l
left a comment
ruocco-l
left a comment
There was a problem hiding this comment.
Just a NIT on a comment
| * `RUN_ALL_PLATFORM_TESTS` is needed for periodic tests, where there is no `ACTOR_BUILDS` env var to match | ||
| * the tests against - without it, every `testActor` would be filtered out as an actor we didn't build. | ||
| */ |
There was a problem hiding this comment.
This comment is a bit misleading: we do need the RUN_ALL_PLATFORM_TESTS to run the scheduled tests but we also need the TESTER_APIFY_TOKEN for those.
The reason why the || !!RUN_ALL_PLATFORM_TESTS earns its place in this check is because locally we can do test on hardcoded runId, so we don't need the TESTER_APIFY_TOKEN.
The comment you wrote should go next to the shouldRun const in testActor and testStandbyActor
There was a problem hiding this comment.
Good catch, I updated the code.
The reason why the || !!RUN_ALL_PLATFORM_TESTS earns its place in this check is because locally we can do test on hardcoded runId, so we don't need the TESTER_APIFY_TOKEN.
This is really confusing, let's think about how to refactor it. RUN_ALL_PLATFORM_TESTS shouldn't be used as a hack to enabled local tests that don't call Actors. Also without TESTER_APIFY_TOKEN, some runs might not be public (they generally are on our account but for community they might not be, by default they are not public anymore)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JSd2mJ6ZFg8cf7dRg4nTQg
|
Oops forgot to merge this |
Catches the merged copy up with the four commits that landed on github-actions-source master after the import (#57, #59, #60, #63), all of which built the claude-review action. Two new files, nothing else changed upstream. `review.yaml` fetches its instructions over HTTP rather than from a checkout, because a reusable workflow runs with the caller's repo checked out and never gets its own, so the URL had to follow the move to this repo. Its `prompt-ref` default moves from `master` to `v1`: consumers call the workflow at `@v1`, and defaulting the prompt to master would run released workflows against unreleased instructions, which is the skew the tag gate exists to prevent. Also drops `RUN_PLATFORM_TESTS` from pr-build-test. Master removed it in #122 in favour of gating on `TESTER_APIFY_TOKEN`, which that step already sets, so after merging master the variable was config nothing reads. Two notes on the checks: - .github/actionlint.yaml ignores two errors on review.yaml. actionlint bakes in a snapshot of popular actions' interfaces from when the pinned version shipped, and claude-code-action has grown since, so it flags `display_report` and the `conclusion` output as undefined. Both are declared in the action's action.yml at @v1 — verified before suppressing, and the patterns name those two symbols so unrelated bad inputs and outputs in that file still fail. - .github/review-prompt.md is prettier-ignored. Prettier collapses the nested bullet list under "do not visit, fetch, infer, or evaluate the following external links" into one run-on line, changing what the model is told. Keeping it byte-identical also makes re-syncing it a plain copy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SkUZADZ6gzWW4GE6CFMreM
5 participants
Closes #121
Shouldn't be breaking for anyone, only if you would configure
RUN_PLATFORM_TESTSlocally while also passingTESTER_APIFY_TOKEN, unlikelySummary
Removed the
RUN_PLATFORM_TESTSenvironment variable and simplified the platform test gating logic to rely solely on the presence ofTESTER_APIFY_TOKEN. This reduces configuration complexity while maintaining the same functionality.Key Changes
RUN_PLATFORM_TESTSfrom environment variable destructuring inlib/lib.tsdescribefunction'srunIfcondition to check!!TESTER_APIFY_TOKEN || !!RUN_ALL_PLATFORM_TESTSinstead of!!RUN_PLATFORM_TESTS || !!RUN_ALL_PLATFORM_TESTSRUN_ALL_PLATFORM_TESTSis needed for periodic testsRUN_PLATFORM_TESTS=1from all example commandsTESTER_APIFY_TOKENis set, keeping regular unit test runs unaffectedImplementation Details
The change leverages the fact that
TESTER_APIFY_TOKENis already a required dependency for platform tests (needed to instantiate the ApifyClient). By using its presence as the gating condition, we eliminate redundant configuration while maintaining backward compatibility with theRUN_ALL_PLATFORM_TESTSflag for periodic test scenarios where noACTOR_BUILDSenvironment variable is available.https://claude.ai/code/session_01JSd2mJ6ZFg8cf7dRg4nTQg