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

github: extract test file as element of matrix #636

Merged
merged 1 commit into from
Mar 27, 2024

Conversation

kenhys
Copy link
Contributor

@kenhys kenhys commented Mar 25, 2024

Before:

test.sh execute multiple test files.
This behavior make it hard to distinct failure test cases.

After:

Use test.sh as launch script. It doesn't execute multiple test files.
This change was introduced to make it easy to distinct each test
cases on error.

@daipom
Copy link
Contributor

daipom commented Mar 25, 2024

I have one concern.
Wouldn't it make running tests in the local environment difficult?

I don't run tests on my local these days.
This concern may be unnecessary. I have commented on this just in case.

Come to think of it, I was thinking that I could use grouping log lines feature to make the results easier to understand before.

I thought that this could be used as follows:

for test_filename in ${test_filenames[@]}; do
    echo -e "::group::Run test: $test_filename\n"
    ...
    echo "::endgroup::"
done

@daipom
Copy link
Contributor

daipom commented Mar 25, 2024

I have one concern. Wouldn't it make running tests in the local environment difficult?

I don't run tests on my local these days. This concern may be unnecessary. I have commented on this just in case.

Sorry, I don't think we need to worry too much about this point.
When I run tests locally, I'm likely to run specific tests repeatedly.
It may be enough to use CI to run all tests.

@kenhys
Copy link
Contributor Author

kenhys commented Mar 25, 2024

Thanks,

Wouldn't it make running tests in the local environment difficult?

It may be. so it takes into account by $CI.

@kenhys
Copy link
Contributor Author

kenhys commented Mar 25, 2024

Come to think of it, I was thinking that I could use grouping log lines feature to make the results easier to understand before.

I didn't know such a feature, Thanks!

@kenhys kenhys force-pushed the pipelines branch 5 times, most recently from 005e3b8 to 1ec1e3e Compare March 25, 2024 09:35
@kenhys kenhys marked this pull request as ready for review March 25, 2024 11:12
@kenhys kenhys force-pushed the pipelines branch 2 times, most recently from 6e94afb to c4daf65 Compare March 26, 2024 01:36
@kenhys
Copy link
Contributor Author

kenhys commented Mar 26, 2024

re-run stalled jobs.

@kenhys kenhys requested a review from ashie March 26, 2024 09:39
Copy link
Contributor

@daipom daipom left a comment

Choose a reason for hiding this comment

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

LGTM, basically!
I just commented on a little thing.

fluent-package/apt/systemd-test/test.sh Outdated Show resolved Hide resolved
Before:

  test.sh execute multiple test files.
  This behavior make it hard to distinct failure test cases.

After:

  * Use test.sh as launch script. It doesn't execute multiple test files.
  This change was introduced to make it easy to distinct each test
  cases on error.
  * Use grouping log lines feature
    See
    https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#grouping-log-lines

NOTE: If you want to test manually on VM, execute each
 test script separately on it.

Signed-off-by: Kentaro Hayashi <[email protected]>
@kenhys kenhys merged commit f41c9e3 into fluent:master Mar 27, 2024
61 checks passed
@kenhys kenhys deleted the pipelines branch March 27, 2024 04:47
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