Skip to content

[github] Fix test log path of run-onecc-build#15472

Merged
seanshpark merged 1 commit intoSamsung:masterfrom
seanshpark:gh_oneccbld_fix_log
May 28, 2025
Merged

[github] Fix test log path of run-onecc-build#15472
seanshpark merged 1 commit intoSamsung:masterfrom
seanshpark:gh_oneccbld_fix_log

Conversation

@seanshpark
Copy link
Copy Markdown
Contributor

This will fix test log path of run-onecc-build workflow.

ONE-DCO-1.0-Signed-off-by: SaeHie Park saehie.park@gmail.com

@seanshpark seanshpark force-pushed the gh_oneccbld_fix_log branch from dc505d0 to e69e56c Compare May 28, 2025 01:48
@seanshpark seanshpark requested a review from a team May 28, 2025 01:49
Copy link
Copy Markdown
Contributor

@shs-park shs-park left a comment

Choose a reason for hiding this comment

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

I left a question, PTAL
=)

Comment thread .github/workflows/run-onecc-build.yml Outdated
path: |
${{ env.NNCC_WORKSPACE }}/compiler/**/*.log
${{ env.NNCC_INSTALL_PATH }}/test/**/*.log
${NNCC_INSTALL_PATH}/test/**/*.log
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AFAIK, single-brace variables (e.g., $VAR) can behave similarly to double-brace expressions (e.g., ${{ env.VAR }}) in some contexts, such as within bash shell scripts (run: blocks).

However, in GitHub Actions-specific contexts like path:, it's recommended to use double braces with the env. prefix (e.g., ${{ env.VAR }}) to ensure proper evaluation.

Did you perform to test whether this works correctly in the path: context?
I'm asking because I'm not entirely sure about its behavior there.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
${NNCC_INSTALL_PATH}/test/**/*.log
${{ env.NNCC_INSTALL_PATH }}/test/**/*.log

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Did you perform to test whether this works correctly in the path: context?

https://github.com/Samsung/ONE/actions/runs/15289092665/job/43005157626?pr=15428
is slightly different but should work with ${NNCC_INSTALL_PATH}
as above name: Test(Release) step uses ${NNCC_INSTALL_PATH} which I've followed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't check explictly about ${NNCC_INSTALL_PATH} is identical with ${{ env.NNCC_INSTALL_PATH }} but
I'd like to follow usage with above step.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The above name: Test(Release) step uses ${NNCC_INSTALL_PATH} in the run: section, which executes a Bash script.
This might behave slightly differently when used in the path: field.

Anyway, I'm curious whether they behave the same.
Let’s find out once it’s landed. 😊

shs-park
shs-park previously approved these changes May 28, 2025
Copy link
Copy Markdown
Contributor

@shs-park shs-park left a comment

Choose a reason for hiding this comment

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

LGTM!
=)

Comment thread .github/workflows/run-onecc-build.yml Outdated
path: |
${{ env.NNCC_WORKSPACE }}/compiler/**/*.log
${{ env.NNCC_INSTALL_PATH }}/test/**/*.log
${NNCC_INSTALL_PATH}/test/**/*.log
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The above name: Test(Release) step uses ${NNCC_INSTALL_PATH} in the run: section, which executes a Bash script.
This might behave slightly differently when used in the path: field.

Anyway, I'm curious whether they behave the same.
Let’s find out once it’s landed. 😊

@seanshpark
Copy link
Copy Markdown
Contributor Author

I'll run some quick test with #15428 :)

@seanshpark
Copy link
Copy Markdown
Contributor Author

output looks

     path: build/compiler/**/*.log
  ${NNCC_INSTALL_PATH}/test/**/*.log

which doesn't look correct

@seanshpark
Copy link
Copy Markdown
Contributor Author

with ${{ ... }} ,

     path: build/compiler/**/*.log
  build/install/test/**/*.log

This will fix test log path of run-onecc-build workflow.

ONE-DCO-1.0-Signed-off-by: SaeHie Park <saehie.park@gmail.com>
Co-authored-by: Seungho Henry Park <shs.park@samsung.com>
Copy link
Copy Markdown
Contributor

@shs-park shs-park left a comment

Choose a reason for hiding this comment

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

👍

@seanshpark seanshpark merged commit bd8b768 into Samsung:master May 28, 2025
7 checks passed
@seanshpark seanshpark deleted the gh_oneccbld_fix_log branch May 28, 2025 04:33
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