Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(run): update args parsing logic #3030

Merged
merged 3 commits into from
Oct 11, 2024
Merged

Conversation

zwhitfield3
Copy link
Contributor

@zwhitfield3 zwhitfield3 commented Oct 9, 2024

Description

Work Item

This PR fixes a list of edge cases for argument parsing within heroku run and heroku local:run.

  1. Reorders the argv elements in a new array using the order that the user inputted. Since this updated logic needs to be applied to both heroku run and heroku local:run, the logic has been refactored into a utility function.
  2. Flags before the -- flag separator are not consumed (fixing flag collisions)
  3. Complete commands are now being passed to the respective options for Dyno and foreman()

Testing

NOTE: I recommend commenting out the commands' functionality after consuming the args as you can see the bug without having to spin up a dyno. We will also need to do a refactor for this fix once we migrate to oclif v4 as some of the fix's functionality will no longer be necessary.

  1. Pull down branch
  2. yarn it up
  3. Log command and argv downstream in both commands
  4. Run the commands with additional args like so:
    ./bin/run run -a testing-deploy -- ./print-args.sh --flag1 val1 --flag1 val2
  5. Confirm that argv maintains the sorted array due to the oclif dependency. (This is where the bug should be noticeable)
    example: argv [ './print-args.sh', '--flag1', '--flag1', 'val1', 'val2' ]
  6. Confirm that command reorders the array to maintain the same order you used when executing the command.
    example: command [ './print-args.sh', '--flag1', 'val1', '--flag1', 'val2' ]
  7. Run the following commands and ensure that the inputted values are in the correct order as executed, there are no duplicate values shown from before the -- separator, and commands are fully captured to their respective functions like dyno opts and foreman()
  • Commands to run:
    Command 1
    ./bin/run run EXAMPLE_READ_ONLY= python -a example-app-prod -- example.py example_file --file="https://*" --> check that full command includes the environment variable (EXAMPLE_READ_ONLY=)
    Command 2
    ./bin/run run -e DOMAIN=test -a test-app -- ruby -e 'puts ENV["ENV_KEY"]' --> check for duplicates for the -e flag
    Command 3
    ./bin/run local:run bin/migrate -- ./print-args.sh --flag1 val1 --flag1 val2 --> check --flag1 is still captured in command given it is given after the -- separator

@zwhitfield3 zwhitfield3 marked this pull request as ready for review October 9, 2024 20:29
@zwhitfield3 zwhitfield3 requested a review from a team as a code owner October 9, 2024 20:29
Copy link
Contributor

@justinwilaby justinwilaby left a comment

Choose a reason for hiding this comment

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

:shipit:

@zwhitfield3 zwhitfield3 merged commit 6850e65 into main Oct 11, 2024
8 checks passed
@zwhitfield3 zwhitfield3 deleted the zw/fix-args-consumption branch October 11, 2024 18:47
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