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

Create an initial stable-rc report email in KCIDB #532

Open
Tracked by #393
padovan opened this issue Jun 19, 2024 · 18 comments
Open
Tracked by #393

Create an initial stable-rc report email in KCIDB #532

padovan opened this issue Jun 19, 2024 · 18 comments
Assignees

Comments

@padovan
Copy link

padovan commented Jun 19, 2024

We need to create a report for our ongoing work for of reporting stable-rc results with Greg KH.

The report will be sent to ourselves first. @crazoes is the main recipient.

Info given by @spbnick:

Here's a sample of notifications we send to Mark. It's sent an hour after last update to the revision.

Here's the code of his subscription: https://github.com/kernelci/kcidb/blob/main/kcidb/monitor/subscriptions/mark_brown.py

Here's the template entry point for notification body for revisions: https://github.com/kernelci/kcidb/blob/main/kcidb/templates/revision_description.txt.j2 (edited)

@JenySadadia
Copy link
Collaborator

Hello @crazoes

Here is the sample report generated for stable-rc from recent submissions to KCIDB:

Subject: stable-rc report for linux-stable-rc.git:linux-6.6.y@v5.16-rc2-19-g383a44aec91c


Below is the summary of results Kernel CI database has recorded
for this revision so far. See complete and up-to-date report at:

    https://kcidb.kernelci.org/d/revision/revision?orgId=1&var-git_commit_hash=580e509ea1348fc97897cf4052be03c248be6ab6&var-patchset_hash=

OVERVIEW

        Builds: ❌ FAIL
         Tests: ✅ PASS

REVISION

    Status
        ✅ PASS
    Commit
        name: v5.16-rc2-19-g383a44aec91c
        hash: 580e509ea1348fc97897cf4052be03c248be6ab6
    Checked out from
        https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.6.y
    By
        broonie, maestro

BUILDS

    Status
             ❌ 1  ✅ 1
    Architectures
        arm  ❌ 1  ✅ 1
    Failures
        ❌ 1  arm multi_v7_defconfig
    By
        broonie, maestro

TESTS

    Status
        ✅ 1
    By
        maestro

See complete and up-to-date report at:

    https://kcidb.kernelci.org/d/revision/revision?orgId=1&var-git_commit_hash=580e509ea1348fc97897cf4052be03c248be6ab6&var-patchset_hash=

LEGEND

    ❌ FAIL     - Failed. Tested code is likely faulty.
    💥 ERROR    - Aborted. Test, tested code, or both might be faulty.
    🟩 MISS     - Missing. Planned, but failed to execute.
    ✅ PASS     - Passed. Tested code is likely correct.
    🆗 DONE     - Finished. Status of tested code is unknown.
    ⏩ SKIP     - Skipped. Planned, but didn't apply.
    ❓ UNKNOWN  - In progress, or status unknown.

    🚧 WAIVED   - Waived result. Test is too new or shows known failures.

    ➖ BLANK    - No data, zero.

The format I have used is from an existing subscription.
Please let me know the preferred report format and I'll update the templates accordingly.

@crazoes
Copy link

crazoes commented Jul 8, 2024

@JenySadadia

Subject is incorrect - it says the report is for 6.6 kernel but the revision is 5.16-rc2-19-g383a44aec91c

In the Revision, Build and Tests section - remove status and By sections as they don't give any additional information.

though, we can have status section under tests but it should be placed under the test name which we want to add here. For example, one of the tests that we will have here is a Boot test and under that we should give a summary with the status tag

@JenySadadia
Copy link
Collaborator

JenySadadia commented Jul 15, 2024

Hello,

Below is the updated report with the modifications as per review discussion:

Subject: KernelCI report for linux-stable-rc:linux-6.6.y@v5.16-rc2-19-g383a44aec91c

OVERVIEW

        Builds: ❌ FAIL
         Tests: ❌ FAIL

REVISION

    Commit
        name: v5.16-rc2-19-g383a44aec91c
        hash: 580e509ea1348fc97897cf4052be03c248be6ab6
    Checked out from
        https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.6.y
    Tested-by
        broonie, maestro

BUILDS

    Summary
        ❌ 2  ✅ 1

    Failures
       -arm (multi_v7_defconfig)
       -intel (multi_v7_defconfig)
       Tested-by: maestro


TESTS

    Summary
        ❌ 3

    Failures
      -kselftest.dt.dt_test_unprobed_devices_sh
      Tested-by: broonie

      -kver
      -kselftest.cpufreq
      -kunit.exec.total_mapping_size_test
      Tested-by: maestro


See complete and up-to-date report at:

    https://kcidb.kernelci.org/d/revision/revision?orgId=1&var-git_commit_hash=580e509ea1348fc97897cf4052be03c248be6ab6&var-patchset_hash=

Here are the addressed review modifications:

  • Updated subject to drop .git
  • Dropped revision status
  • Removed KCIDB result link from the top of the report
  • Renamed By section with Tested-by:
  • Updated report to only describe details of failures
  • Updated format for presenting build info such as arch and config options
  • Updated test failure info to mention the whole test path
  • Grouped all the results based on CI origin
  • Dropped KCIDB legends information

Note: I am working on describing failure/error info. We'll discuss Summary part once everything else is done.

Please let me know your thoughts @crazoes. Please note that I have generated report on dummy data so please do not verify/validate 😅

@padovan
Copy link
Author

padovan commented Jul 15, 2024

Tested-by: is a confusing, as that is also a kernel commit tag we use a lot. Test systems, Test data sources, etc?

@JenySadadia
Copy link
Collaborator

Tested-by: is a confusing, as that is also a kernel commit tag we use a lot. Test systems, Test data sources, etc?

I think Greg suggested @crazoes to use Tested-by tag.

@JenySadadia
Copy link
Collaborator

Revised report with build errors:

Subject: KernelCI report for linux-stable-rc:linux-6.6.y@v5.16-rc2-19-g383a44aec91c

OVERVIEW

        Builds: ❌ FAIL
         Tests: ❌ FAIL

REVISION

    Commit
        name: v5.16-rc2-19-g383a44aec91c
        hash: 580e509ea1348fc97897cf4052be03c248be6ab6
    Checked out from
        https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.6.y
    Tested-by
        broonie, maestro

BUILDS

    Summary
        ❌ 2  ✅ 1

    Failures
       -arm (multi_v7_defconfig)
       Build error: display/dc/link/link_factory.c:743:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
       -intel (multi_v7_defconfig)
       Build error: make: *** [Makefile:1996: modules] Error 1
       Tested-by: maestro


TESTS

    Summary
        ❌ 3

    Failures
      -kselftest.dt.dt_test_unprobed_devices_sh
      Tested-by: broonie

      -kver
      -kselftest.cpufreq
      -kunit.exec.total_mapping_size_test
      Tested-by: maestro


See complete and up-to-date report at:

    https://kcidb.kernelci.org/d/revision/revision?orgId=1&var-git_commit_hash=580e509ea1348fc97897cf4052be03c248be6ab6&var-patchset_hash=

@crazoes
Copy link

crazoes commented Jul 16, 2024

Hi @JenySadadia

This report looks much better than previous one.

Some more changes which I think we need to make :-

Subject is still incorrect, not sure why we are getting the branch name as 6.6 if we are looking at results of 5.16. So we need to fix that before everything else.

I think we can also make the subject better. I'm thinking of something like this :-
KernelCI report for stable-rc: linux-5.16.y@v5.16-rc2-19-g383a44aec91c

In the overview section, let's also mention the number of build failures and the number of test failures otherwise it gives a little wrong impression of everything failing.

In my opinion, we should remove Tested-by tag. @JenySadadia there seems to be a little confusion about the tested by tag here. What we want is to have a tag Tested-by: kernelci.org bot <bot@kernelci.org> at the end of the report. This is what Greg wanted from us and we shouldn't really mention maestro, broonie etc. @padovan what are your thoughts about this?

I'm expecting revision to look like this :-

REVISION

    Commit
        name: v5.16-rc2-19-g383a44aec91c
        hash: 580e509ea1348fc97897cf4052be03c248be6ab6
    Checked out from
        https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.16.y

Build section looks really good to me but some more changes that I'd like to see are :-

  • Remove the summary under the build and also tested-by, it doesn't really give much info to the users. Let's only keep the following
   Failures
       -arm (multi_v7_defconfig)
       Build error: display/dc/link/link_factory.c:743:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
       -intel (multi_v7_defconfig)
       Build error: make: *** [Makefile:1996: modules] Error 1

FYI, there isn't any architecture named intel so something is wrong here. It might be x86_64 so please verify that too.

Test section is really confusing and it's hard to read it. Let's remove the differentiation of the results from different CI systems. So we shouldn't keep the Tested-by tag in the Test section as well. All the results should be bundled together, no matter from which CI systems it comes from.

@JenySadadia for stable-rc we just want to send the boot test results which is baseline.login tests. So under the Test section we want something like this :-

TESTS

    Failures
      baseline.login (boot test)
      - Device1 (config_file_name)
      - Device2 (config_file_name)
      ....

Again, let's remove the summary from it as it doesn't really provide any info

At the end of everything, we want to have the following :-

Tested-by: kernelci.org bot <bot@kernelci.org>

Thanks,
KernelCI Team

@padovan
Copy link
Author

padovan commented Jul 16, 2024

yes, it is exactly what @crazoes said about the Tested-by tag.

@JenySadadia
Copy link
Collaborator

Hello @crazoes

Thanks a lot for all the suggestions.

Subject is still incorrect, not sure why we are getting the branch name as 6.6 if we are looking at results of 5.16. So we need to fix that before everything else.

As I stated earlier, the report I generated was on dummy data that's why it was showing incorrect data. I fixed it to have correct output.

I think we can also make the subject better. I'm thinking of something like this :-
KernelCI report for stable-rc: linux-5.16.y@v5.16-rc2-19-g383a44aec91c

Okay. Addressed this.

In the overview section, let's also mention the number of build failures and the number of test failures otherwise it gives a little wrong impression of everything failing.

Makes sense. Updated to have number of passed and failed builds/tests. Also, dropped summary sections for BUILDS and TESTS as it's already been covered in Overview section.

In my opinion, we should remove Tested-by tag. @JenySadadia there seems to be a little confusion about the tested by tag here. What we want is to have a tag Tested-by: kernelci.org bot bot@kernelci.org at the end of the report.

Oh, sorry for the confusion. I fixed the Tested-by tag.

@JenySadadia for stable-rc we just want to send the boot test results which is baseline.login tests.

Why are we only interested in boot tests? Bdw I have updated the section to have arch(configs). It's not possible atm to list down device names as KCIDB doesn't have a dedicated field for it. Maestro uses misc.platform but that's not generic to all CI systems. On a separate note, the discussion to add platform to KCIDB schema has already been started.

Let's remove the differentiation of the results from different CI systems. So we shouldn't keep the Tested-by tag in the Test section as well. All the results should be bundled together, no matter from which CI systems it comes from.

KCIDB generates an aggregate report from different CI systems' results. I thought it's worth mentioning which CI system reported which failure. Hence, I kept By sections for build and test failures. But again it depends on how the report is going to be used. What are your thoughts on that @padovan?

Here is the updated report:

Subject: KernelCI report for stable-rc: linux-6.6.y@v6.6.35-193-g580e509ea1348

OVERVIEW

        Builds: 1 passed, 3 failed

         Tests: 0 passed, 4 failed


REVISION

    Commit
        name: v6.6.35-193-g580e509ea1348
        hash: 580e509ea1348fc97897cf4052be03c248be6ab6
    Checked out from
        https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.6.y
    By
        broonie, maestro

BUILDS

    Failures
       -arm (multi_v7_defconfig)
       Build error: display/dc/link/link_factory.c:743:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
       -arm (vexpress_defconfig)
       Build error: display/dc/link/link_factory.c:743:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
       -i386 (allnoconfig)
       Build error: make: *** [Makefile:1996: modules] Error 1
       By: maestro


TESTS

    Boot failures
      arm:(omap2plus_defconfig)
      By: broonie

      arm:(multi_v7_defconfig, vexpress_defconfig)
      i386:(allnoconfig)
      By: maestro


See complete and up-to-date report at:

    https://kcidb.kernelci.org/d/revision/revision?orgId=1&var-git_commit_hash=580e509ea1348fc97897cf4052be03c248be6ab6&var-patchset_hash=


Tested-by: kernelci.org bot <bot@kernelci.org>

Thanks,
KernelCI team

@padovan
Copy link
Author

padovan commented Jul 17, 2024

By: broonie

By seems a bit unclear. Maybe Test system?

On Build failures, may we need a black line between each failure?

@JenySadadia
Copy link
Collaborator

By: broonie

By seems a bit unclear. Maybe Test system?

We can use Reported-by: or maybe CI: ?

On Build failures, may we need a black line between each failure?

Yes, I'll add it.

@JenySadadia
Copy link
Collaborator

Oh, yes, I remember, I kept a blank line after a group of CI results. For example,

BUILDS

    Failures
       -arm (omap2plus_defconfig)
       By: broonie

       -arm (multi_v7_defconfig)
       Build error: display/dc/link/link_factory.c:743:1: error: the frame si=
ze of 1040 bytes is larger than 1024 bytes [-Werror=3Dframe-larger-than=3D]
       -arm (vexpress_defconfig)
       Build error: display/dc/link/link_factory.c:743:1: error: the frame si=
ze of 1040 bytes is larger than 1024 bytes [-Werror=3Dframe-larger-than=3D]
       -i386 (allnoconfig)
       Build error: make: *** [Makefile:1996: modules] Error 1
       By: maestro

@spbnick
Copy link
Collaborator

spbnick commented Jul 17, 2024

A side note: we need to credit origins when we show and send results, so let's not drop them, but work on improving representation instead. We should not take credit over the resources supplied by other CI systems, but highlight them instead.

Jeny, as you work on those changes, could you create separate templates for stable-rc instead of changing the stock ones? And use and expand the template macro libraries, as necessary? This way we could get this merged sooner, and then we can work on integrating what works in the main reports.

@spbnick
Copy link
Collaborator

spbnick commented Jul 17, 2024

And one more thing: we should avoid putting too much information into the (plain text or even HTML) emails. It quickly becomes unusable as the only controls you have to review the data are scrolling back and forth and string search.

I think the emails should act as an alert system, first of all, presenting the most important data succinctly, and directing people towards dashboards, which should have better tools for exploring and representing the information. It should usually also have more data by the time the email is viewed, as we don't really send them when we're "done", because we're potentially never done testing.

As a perspective, KernelCI legacy reports were already unreadable due to the amount of data in them. Now we're adding data from other CI systems, and so that approach wouldn't work at all.

@JenySadadia
Copy link
Collaborator

Jeny, as you work on those changes, could you create separate templates for stable-rc instead of changing the stock ones? And use and expand the template macro libraries, as necessary? This way we could get this merged sooner, and then we can work on integrating what works in the main reports.

Yes, I have already created a bunch of stable_rc_*.j2 templates for this.
See #537.

@JenySadadia
Copy link
Collaborator

Hello,

Here is an updated report following our discussion yesterday:

Subject: KernelCI report for stable-rc: linux-6.6.y@v6.6.35-193-g580e509ea1348

OVERVIEW

        Builds: 0 passed, 4 failed

         Tests: 0 passed, 5 failed

           CIs: broonie, maestro

REVISION

    Commit
        name: v6.6.35-193-g580e509ea1348
        hash: 580e509ea1348fc97897cf4052be03c248be6ab6
    Checked out from
        https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-6.6.y


BUILDS

    Failures
       -arm (omap2plus_defconfig)
       CI: broonie

       -arm (multi_v7_defconfig)
       Build error: display/dc/link/link_factory.c:743:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
       -arm (vexpress_defconfig)
       Build error: display/dc/link/link_factory.c:743:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
       -x86 (allnoconfig)
       Build error: make: *** [Makefile:1996: modules] Error 1
       CI: maestro


BOOT TESTS

    Failures
      arm:(vexpress_defconfig)
      -bcm2711-rpi-4-b
      x86:(allnoconfig)
      -acer-cb317-1h-c3z6-dedede
      CI: maestro


See complete and up-to-date report at:

    https://kcidb.kernelci.org/d/revision/revision?orgId=1&var-git_commit_hash=580e509ea1348fc97897cf4052be03c248be6ab6&var-patchset_hash=


Tested-by: kernelci.org bot <bot@kernelci.org>

Thanks,
KernelCI team

ChangeLog:

  • Renamed all the By sections to CI
  • Moved list of origins from REVISION to OVERVIEW
  • Renamed TESTS section to BOOT TESTS
  • Displayed device list for boot tests (working only for maestro atm)

@padovan
Copy link
Author

padovan commented Jul 22, 2024

I'd use CI systems: rather than just CIs. Other than that this seems enough to deploy imo and then we iterate on improving it as we go.

@crazoes
Copy link

crazoes commented Jul 22, 2024

@JenySadadia apart from what @padovan said, last time we also discussed adding the grafana dashboard link pointing to the failures in the build section. It will be great if you can add that too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

4 participants