Skip to content

Conversation

@linlin-s
Copy link
Contributor

@linlin-s linlin-s commented Jan 13, 2023

Signed-off-by: Linlin Sun linls@amazon.com

Issue #, if available:
#465

Description of changes:
This PR adds a workflow PR-content-check to help us check whether there is any source code changes in the incoming PR. If the changed is detected, the workflow will be completed and executed successfully. When PR-content-check workflow is completed successfully, ion-test-driverand ion-java-regression-detection workflows will be triggered. If the PR-content-check is failed, ion-test-driver and ion-java-regression-detection will not be executed.
This PR also updated the version of actions/checkout

Test:

image

ion-test-driver and ion-java-regression-detection were skipped based on the execution status of PR-content-check
image

image

ion-test-driver and ion-java-regression-detection were triggered based on the execution status of PR-content-check
image

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

Copy link
Contributor

@tgregg tgregg left a comment

Choose a reason for hiding this comment

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

Nice! Can we also perform this check before running the ion-test-driver workflow?

@codecov
Copy link

codecov bot commented Jan 13, 2023

Codecov Report

Base: 66.59% // Head: 66.60% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (31259b2) compared to base (1cac7fe).
Patch has no changes to coverable lines.

❗ Current head 31259b2 differs from pull request most recent head 51f657b. Consider uploading reports for the commit 51f657b to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #467   +/-   ##
=========================================
  Coverage     66.59%   66.60%           
  Complexity     5416     5416           
=========================================
  Files           156      156           
  Lines         22751    22751           
  Branches       4105     4105           
=========================================
+ Hits          15152    15153    +1     
  Misses         6236     6236           
+ Partials       1363     1362    -1     
Impacted Files Coverage Δ
src/com/amazon/ion/impl/BlockedBuffer.java 51.58% <0.00%> (+0.12%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@linlin-s
Copy link
Contributor Author

Nice! Can we also perform this check before running the ion-test-driver workflow?

Yes, it is doable. It might require some minor change on ion-test-driver workflow too. I'll look into that and make the update.

id: check-content
run: |
cd ion-java-new
if [[ $(git log -1 --name-only) == *"src/"* ]]; then echo "result=pass"; else exit 1; fi No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

Is failing the workflow the only way to achieve the behavior we want? Ideally this job would always succeed, and we'd simply convey the result to the other jobs so to determine if they should run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This not the only way. We can add the steps of PR-content-check to both ion-java-regression-detection.yml and ion-test-driver.yml. Since both of the share the same logic of checking PR content, I'd prefer to reuse the workflow PR-content-check.yml in both applicable workflows to avoid duplication.

@linlin-s
Copy link
Contributor Author

Updates after commit Update workflows to avoid failing PR-content-check workflow:
Both ion-test-driver and ion-java-performance-regression-detection will be triggered by pull_request. Within both workflows, the first job is PR-content-check which helps us to determine whether we should execute the following jobs(ion-java-performance-regression-detection or ion-test-driver). Here are examples of how PR-content-check works in these process:
PR contains no source code change:
image
image

PR contains source code change:
image
image

Note: The reason this PR failed the workflows ion-java-performance-regression and ion-test-driver is PR-content-check.yml has not been merged to the master branch yet.

Copy link
Contributor

@tgregg tgregg left a comment

Choose a reason for hiding this comment

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

Excellent, this is a big improvement.

…coming pull request.

Signed-off-by: Linlin Sun <linls@amazon.com>
Update the trigger events of ion-test-driver and ion-java-performance-regression-detection workflows.

Update the output of PR-content-check.
@linlin-s linlin-s merged commit de540a2 into amazon-ion:master Jan 23, 2023
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