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

ci: add generate log files and upload to S3 bucket #1194

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sky1122
Copy link

@sky1122 sky1122 commented Nov 20, 2024

  • add upload to S3 bucket

    • e2e-linux.yaml
    • e2e-macos.yaml
    • e2e-windows.yaml
  • generate test log report while running the e2e testing

Makefile

  • [ X] I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sky1122 sky1122 marked this pull request as ready for review November 27, 2024 17:01
@sky1122 sky1122 requested a review from a team as a code owner November 27, 2024 17:01
@sky1122 sky1122 force-pushed the ci-e2e-testing branch 2 times, most recently from 3d47915 to c44fe8f Compare December 2, 2024 19:58
@sky1122 sky1122 requested a review from austinvazquez December 9, 2024 18:44
Copy link
Member

@austinvazquez austinvazquez left a comment

Choose a reason for hiding this comment

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

Overall LGTM, but one question do we want to have reports generated on every pull request? Does it make sense to limit it to merges to mainline and nightly runs?

with:
role-to-assume: ${{ secrets.ROLE }}
role-session-name: credhelper-test
aws-region: ${{ secrets.REGION }}
aws-region: ${{ secrets.REGION }}
Copy link
Member

Choose a reason for hiding this comment

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

nit: here and a few other places appears to be some trailing whitespace being appended to aws-region line.

Copy link
Author

Choose a reason for hiding this comment

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

make sense. I add new commands in makefile and only let push event and nightly runs to generate the test report.

Copy link
Author

Choose a reason for hiding this comment

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

For merges to mainline, I summarize two conditions. If there are more conditions, please let me know.

if [ "${{ github.event_name }}" == "push" ] && [ "${{ github.ref }}" == "refs/heads/main" ] || 
   [ "${{ github.event_name }}" == "pull_request" ] && [ "${{ github.base_ref }}" == "main" ]; then

@sky1122 sky1122 requested a review from a team as a code owner December 16, 2024 21:52
@sky1122 sky1122 force-pushed the ci-e2e-testing branch 3 times, most recently from 379dcc7 to d197701 Compare December 17, 2024 01:50
@sky1122 sky1122 requested review from austinvazquez and removed request for austinvazquez December 18, 2024 21:55
austinvazquez
austinvazquez previously approved these changes Dec 31, 2024
Copy link
Member

@austinvazquez austinvazquez left a comment

Choose a reason for hiding this comment

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

LGTM on CI pass.

Signed-off-by: Jingwei Wang <[email protected]>

fix Amazon Linux if condition

Signed-off-by: Jingwei Wang <[email protected]>

ci: only merges to mainline and nightly runs will generate log reports

Signed-off-by: Jingwei Wang <[email protected]>

Squashed commit message for the first 10 commits

Signed-off-by: Jingwei Wang <[email protected]>

fix minor errors

Signed-off-by: Jingwei Wang <[email protected]>

fix minor errors

Signed-off-by: Jingwei Wang <[email protected]>

fix minor errors

Signed-off-by: Jingwei Wang <[email protected]>

fix minor errors

Signed-off-by: Jingwei Wang <[email protected]>

fix minor errors

Signed-off-by: Jingwei Wang <[email protected]>
Signed-off-by: Jingwei Wang <[email protected]>
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