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

Terratest readable output #826

Merged
merged 19 commits into from
Jan 20, 2022
Merged

Conversation

AugustasV
Copy link
Contributor

#825 Updated make file and terratest files to output terratest results.
In that way it will be easier to debug.

Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

@AugustasV thanks a lot for your contribution.

We also have .github/workflows/upgrade-testing.yaml which involves terratest execution so it is a candidate for the enhancement.

Please also sign off the commits to satisfy DCO

Copy link
Member

@jkremser jkremser left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! pls see the comments, otherwise it looks good

@AugustasV AugustasV force-pushed the terratest_readable_output branch 2 times, most recently from dd86134 to 58fa005 Compare January 6, 2022 19:01
@AugustasV AugustasV requested review from ytsarev and jkremser January 6, 2022 19:02
@AugustasV AugustasV force-pushed the terratest_readable_output branch from 03bc83b to 3245aab Compare January 6, 2022 19:50
@jkremser
Copy link
Member

jkremser commented Jan 7, 2022

cool 💯 , I can see the uploaded zip file with the separated log files: https://github.com/k8gb-io/k8gb/actions/runs/1664623772
Bummer, that it's not reachable directly from link under pull-request, but I had to go via actions tab.

Could you pls add one more thing? The link that would lead to that web page where it can be get.

this should be the filed - ${{ github.run_id }}
so pls put something like

echo Separated log files will be available under artifact section at https://github.com/k8gb-io/k8gb/actions/runs/${{ github.run_id }}

to Terratest log parser step

example for PR msg:
https://github.com/k8gb-io/k8gb/blob/master/.github/workflows/olm_pr.yaml#L94

and it then looks like this:
k8s-operatorhub/community-operators#499

Another future improvement could be actually printing these logs right away during the build, but let's not complicate it even more. There is a way to make the collapsible log sections like in TravisCi - https://github.yoyoweb123.workers.devmunity/t/has-github-action-somthing-like-travis-fold/16841

so then having something like

Terratest logs:
▶ TestK8gbSplitFailoverExample
▶ TestK8gbIngressAnnotationRR
..

would be awesome

@AugustasV
Copy link
Contributor Author

I tried to implement collapsible logs but without success. It collapsed everything. It is probably not possible to collapse one by one. Any ideas?
image

@AugustasV AugustasV force-pushed the terratest_readable_output branch from a430ccc to 3f5d741 Compare January 13, 2022 16:35
@jkremser
Copy link
Member

right, nice so it works almost :D, I think we need to iterate through the files one by one and "cat"em within each collapsible section, check the suggestion comments with the diff I've made

as for the DCO check, could you pls sign them all? It's still possible with interactive rebase(tutorial). Or you can make your life easier and squash all the commits to just one, up to you, or we can squash it during the PR merge using "github button".

Otherwise some tips for signing are also here: https://github.com/k8gb-io/k8gb/blob/master/CONTRIBUTING.md#signature
All projects under CNCF umbrella have to have this policy.

AugustasV and others added 11 commits January 16, 2022 11:54
Signed-off-by: AugustasV <[email protected]>
Signed-off-by: AugustasV <[email protected]>
Signed-off-by: AugustasV <[email protected]>
Signed-off-by: AugustasV <[email protected]>
Signed-off-by: AugustasV <[email protected]>
Signed-off-by: AugustasV <[email protected]>
Signed-off-by: AugustasV <[email protected]>
Signed-off-by: AugustasV <[email protected]>
Signed-off-by: AugustasV <[email protected]>
Signed-off-by: AugustasV <[email protected]>
Signed-off-by: Jirka Kremser <[email protected]>
Signed-off-by: AugustasV <[email protected]>
AugustasV and others added 2 commits January 16, 2022 11:54
Co-authored-by: Jirka Kremser <[email protected]>
Signed-off-by: AugustasV <[email protected]>
Co-authored-by: Jirka Kremser <[email protected]>
Signed-off-by: AugustasV <[email protected]>
@AugustasV AugustasV force-pushed the terratest_readable_output branch from 069cf73 to 0a4d975 Compare January 16, 2022 10:55
@AugustasV AugustasV requested a review from jkremser January 16, 2022 10:57
Copy link
Member

@jkremser jkremser left a comment

Choose a reason for hiding this comment

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

thanks, DCO is passing, we are almost there

@AugustasV AugustasV force-pushed the terratest_readable_output branch 2 times, most recently from 5512a41 to 825f732 Compare January 17, 2022 18:31
@AugustasV AugustasV force-pushed the terratest_readable_output branch from 825f732 to 16506d6 Compare January 17, 2022 18:38
@AugustasV AugustasV force-pushed the terratest_readable_output branch from b43bef7 to 3608be5 Compare January 17, 2022 18:51
@AugustasV
Copy link
Contributor Author

I see that my all DCO signed with git log command, but here I see it's not

Signed-off-by: AugustasV <[email protected]>
@AugustasV AugustasV force-pushed the terratest_readable_output branch from 3608be5 to d8ab44a Compare January 17, 2022 19:21
@jkremser jkremser self-requested a review January 20, 2022 11:37
@jkremser jkremser merged commit ea42eb2 into k8gb-io:master Jan 20, 2022
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