Skip to content

Conversation

@simon-engledew
Copy link

@simon-engledew simon-engledew commented Nov 4, 2020

The method getAnalysisKey would call getWorkflowPath and raise an API Error.

This change follows the pattern in prepareLocalRunEnvironment to set a dummy value for the required environment variable, therefore shortcutting the API request.

I have tried to mirror the existing mechanism for stubbing out GITHUB_JOB as closely as possible.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

The method getAnalysisKey would call getWorkflowPath and raise an API Error.

This change follows the pattern in prepareLocalRunEnvironment to set a dummy value for the required environment variable, therefore shortcutting the API request.
Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

Looks reasonable. So just to confirm, was it previously failing with a "Invalid API call in local run" error?

@simon-engledew
Copy link
Author

Looks reasonable. So just to confirm, was it previously failing with a "Invalid API call in local run" error?

Yes, exactly! 👍 Any attempts to access the API when local run is turned on automatically raise an Error.

testing-utils.ts / setupTests() already stores and restores the env
Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

LGTM

@simon-engledew simon-engledew merged commit 0ed3207 into main Nov 4, 2020
@simon-engledew simon-engledew deleted the simon-engledew/fix-act-local branch November 4, 2020 14:30
@github-actions github-actions bot mentioned this pull request Nov 9, 2020
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