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

Report more warnings, summarize job output #1996

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

Conversation

dmitriy-sobolev
Copy link
Contributor

Addresses #1850, with #1994 being an additional motivator.

This PR adds -Wall (for GCC/Clang/ICX/ICPX) and /W4 (for MSVC) compiler flags to detect various warnings, such as unused variables, dangerous type conversions and other.

The number of warnings is large to the extent it can be challenging to navigate through the log files. To address this, the PR also adds a summary for each CI job to simplify analysis of their outputs. The summary includes:

  • Environment details.
  • A histogram categorizing the types of warnings.
  • Extracted ctest output.


if [[ "${{ matrix.backend }}" == "dpcpp" ]]; then
make_targets="build-onedpl-sycl_iterator-tests build-onedpl-ranges-tests"
ctest_flags="-R (sycl_iterator_.*)|(std_ranges_.*)\.pass"
echo "::warning::dpcpp backend is set. Compile and run only sycl_iterator tests"
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the ::warning syntax is a workflow annotation: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions#setting-a-warning-message.

Do we want to remove these from the logs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It becomes unnecessary. The summary will show the number of the tests, as well as the ctest details if you expand the corresponding section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can check this tab to see how it looks like.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want to remove these from the logs?

Ah, I see. Yes, let me remove it...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@dmitriy-sobolev dmitriy-sobolev added the test Test only Change label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement test Test only Change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants