GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Do not use `core.exportVariable` in unit tests (2nd attempt) by mbg · Pull Request #4212 · github/codeql-action Conversation
Member
This PR revives #3930 as per the discussion at #4198 (comment). This PR aims to accomplish the same goals as #3930, but is updated based on changes that have happened in the codebase since. The original PR description follows. Although we already clear environment variables that are set during unit tests, that does not affect the behaviour of This PR improves the situation by introducing a wrapper around The main change compared to the previous PR is that we now have At first glance, a potential concern with using the Outside of unit tests, where Risk assessmentFor internal use only. Please select the risk level of this change:
Which use cases does this change impact?Environments:
How did/will you validate this change?
If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning
🟡 Changes recommended
1 open findingWhat changed in this PRIntroduces an environment abstraction that prevents unit tests from exporting variables to subsequent workflow steps. Changes:
Files excluded by content exclusion policy (1)
🧠 Review effort: Balanced Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactionsUh oh!
There was an error while loading. Please reload this page.
henrymercer
reviewed
Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page.
mbg
force-pushed
the
mbg/tests/fix-export-variable-2
branch
from
9326334 to
bd97dda
Compare
henrymercer
approved these changes
Comment on lines
+223
to
+227
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this can now be cleaned up
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Comment on lines
+323
to
+326
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update this JSDoc
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was worried I might have missed one
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. Uh oh!
There was an error while loading. Please reload this page.
This was referenced
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
Copilot code review
Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Uh oh!
There was an error while loading. Please reload this page. AssigneesNo one assignedLabels
size/L
ProjectsMilestoneNo milestoneDevelopmentSuccessfully merging this pull request may close these issues. Uh oh!
There was an error while loading. Please reload this page. Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| You are visitor 01402503 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required. |