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 line count for errors on tap output #261

Merged
merged 1 commit into from
Mar 29, 2021

Conversation

skos-ninja
Copy link
Contributor

This fixes the line count being incorrect on the tap output.
The previous code would work but only if the last file was the file that had an error.

@gregswift
Copy link
Contributor

gregswift commented Nov 19, 2020

nm... dug into code.. unrelated i think

@gregswift
Copy link
Contributor

Sorry... dug in so i could figure out the problem and I do think this solves the problem.

Here is the output i'm seeing, which skips step 4, because step 3 was an error

1..7
ok 1 #skip - /data/tmp/rendered/common/control.yaml (RemoteResourceS3)
ok 2 - /data/tmp/rendered/defaults.yaml (ConfigMap)
not ok 3 - /data/tmp/rendered/enabled.yaml (ConfigMap) - annotations: Additional property annotations is not allowed
ok 5 - /data/tmp/rendered/s3.yaml (ConfigMap)
ok 6 #skip - /data/tmp/rendered/s3.yaml (List)
ok 7 #skip - /data/tmp/rendered/storageclass.yaml
not ok 8 - /data/tmp/rendered/versions.yaml (ConfigMap) - annotations: Additional property annotations is not allowed

@skos-ninja
Copy link
Contributor Author

@gregswift yep it fixes exactly that issue

@skos-ninja
Copy link
Contributor Author

@garethr sorry to poke but any chance this could be reviewed and merged if you are happy with it?

@SailingYYC
Copy link

@garethr sorry to poke but any chance this could be reviewed and merged if you are happy with it?

@carlossg Any thoughts on merging?

@carlossg carlossg merged commit c9ced79 into instrumenta:master Mar 29, 2021
@carlossg
Copy link
Collaborator

thanks

@carlossg
Copy link
Collaborator

could you add a test to output_test.go ?

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.

4 participants