fix parameter with space error

[Simpcyclassy]

Signed-off-by: Simpcyclassy [email protected]

What does this change

• Uncommented the tests for parameters with a space.
• Changed code implementation to allow for parameters with a space
• Updated tests that relied on incorrect behaviour.

What issue does it fix

Closes #1862

Notes for the reviewer

We have left the splitting of flags by space but changed the implementation for arguments.

Checklist

• Did you write tests?
• Did you write documentation?
• Did you change porter.yaml or a storage document record? Update the corresponding schema file.
• If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

Reviewer Checklist

• Comment with /azp run test-porter-release if a magefile or build script was modified
• Comment with /azp run porter-integration if it's a non-trivial PR

[carolynvs]

I found that one of the tests is failing because it incorrectly relied on the old bug that you fixed.

Can you take a look at tests/integration/testdata/bundles/outputs-example/porter.yaml? In a few places it passes an argument to the exec mixin that has a space in it

- exec:
      description: "Echo step-level output"
      command: ./helpers.sh
      arguments:
        - "dump {{ bundle.outputs.user }}"

With your fix, I believe this should be changed to

- exec:
      description: "Echo step-level output"
      command: ./helpers.sh
      arguments:
        - dump
        - "{{ bundle.outputs.user }}"

Can you try making that fix to the test data and then pushing up that change to your branch? I think that will fix the integration test that is failing TestStepLevelAndBundleLevelOutputs.

[Simpcyclassy]

Hello @carolynvs sorry we took a long break on this due to personal engagements. We are taking a look at it now and implementing the suggested fix as well as checking other related files.

I am also having a hard time testing this locally so I am mostly using the git pipeline to test this. Have you by any chance encountered this? I am running on Mac 11.6.4

[carolynvs]

What kind of problems are you seeing when running the tests?

This is just a guess but are you are running all the integration tests with mage testintegration and it's super slow? In case that's the problem, here's how to just run a single test:

# first build the code
mage build

# run just the one test
# -tags is used to include files that use the build tag "integration"
# -v turns on verbose test output in go
# -count=1 lets you repeat the test even when you didn't change the code, i.e. do not use the cached test result
# -run provides a substring to match the test name against and only run tests that contain the substring
# ./tests/integration saves time by only looking for matching tests in the directory that contains the test
go test -tags=integration -v -count=1 -run TestInstall_stringParam ./tests/integration

[Simpcyclassy]

This is what I get

[carolynvs]

@Simpcyclassy Looks like your docker environment ran out of disk space (look for no space on device in that screenshot). I find the easiest way to fix that error is to run docker system prune which removes unused volumes, images and containers.

[Simpcyclassy]

I have been able to free up some space and it works fine now.

[carolynvs]

Thanks for fixing this! It's been bothering us since like the beginning of the project and it's great to have it working in time for our 1.0 release. 🎉