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: use FormattedPath [TFCSCM-124] #3509

Merged
merged 1 commit into from
Aug 4, 2022

Conversation

teodora-sandu
Copy link
Contributor

@teodora-sandu teodora-sandu commented Jul 29, 2022

What does this PR do?

This removes the formattedPath function and instead uses the FormattedPath field returned by snyk-iac-test. It also updates the version of snyk-iac-test to https://github.com/snyk/snyk-iac-test/releases/tag/v0.14.0.

Any background context you want to provide?

It contains loads of good fixes.

What are the relevant tickets?

Screenshots

@teodora-sandu
Copy link
Contributor Author

@YairZ101
Copy link
Contributor

Do you have any instructions for this?

No, when I generated this fixture I simply took several files from our goof repo, moved them to a new dir, and scanned the dir.
Do you think we should have a documented procedure to generate this fixture?

@teodora-sandu
Copy link
Contributor Author

@YairZ101 I think documenting it would be good, even if it's just a comment at the top of this file. I assume you ran snyk-iac-test directly on the files to generate that output?

@YairZ101
Copy link
Contributor

YairZ101 commented Aug 1, 2022

@YairZ101 I think documenting it would be good, even if it's just a comment at the top of this file. I assume you ran snyk-iac-test directly on the files to generate that output?

I personally used the TS CLI using debug mode but you can also use snyk-iac-test if you prefer.

@ipapast
Copy link
Contributor

ipapast commented Aug 2, 2022

hey @teodora-sandu , I found the files that Yair used from a different repo.

  1. On the first commit fd5227f I put them into their own fixtures folder, so we can have specific ones to test.

The json tests will not pass of course only with this commit.

  1. Then, I tried to generate the expected results from this file (and trim unnessary things as the fixtures have changed quite a bit) - 8ddd8f3

  2. I am now going to merge the conflicts and get the latest version for snyk-iac-test as it has bug fixes and it also involves the "float" bug you just fixed today - it's in these fixtures.
    Edit: merged conflicts and now using v0.13.1 like master, which includes bug fixes.

  3. Updated the msg field in the experimental output on this commit: a3169a9

Now the tests should pass. (they do 🎉 )

However, have a look again the msg - it looks a bit strange to me that it includes the resource. in front, but I am not up to date with the current formatting. I confirm that this is how it looks in the experimental output though. We could chat about it tomorrow if you like.

@teodora-sandu teodora-sandu marked this pull request as ready for review August 3, 2022 10:14
@teodora-sandu teodora-sandu requested a review from a team as a code owner August 3, 2022 10:14
@teodora-sandu teodora-sandu force-pushed the fix/formatted-path-snyk-iac-test branch from a3169a9 to 68ed18c Compare August 3, 2022 10:15
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