Skip to content

Conversation

JenySadadia
Copy link
Contributor

  • Update kcidb_test_results to fetch architecture and compiler information for tests
  • Take architecture and compiler into account while grouping tests in evaluate_test_results

@JenySadadia JenySadadia requested a review from padovan September 1, 2025 08:02
@JenySadadia JenySadadia force-pushed the tree-report-notification branch 2 times, most recently from 354dc64 to c080a2a Compare September 1, 2025 09:14
Copy link
Collaborator

@MarceloRobert MarceloRobert left a comment

Choose a reason for hiding this comment

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

You need to change the jinja template as well. The new/fixed/unstable issues are destructured there and so you'll need to add another for layer in order for the status to be properly shown.

@MarceloRobert
Copy link
Collaborator

Also please rebase, we have fixed the integration test that was causing an error and we also had some changes in the checkout summary that could conflict with yours

Update `kcidb_test_results` to fetch
architecture and compiler information
for tests.

Signed-off-by: Jeny Sadadia <[email protected]>
@JenySadadia JenySadadia force-pushed the tree-report-notification branch from f608969 to 455e8c0 Compare September 3, 2025 07:14
@JenySadadia JenySadadia force-pushed the tree-report-notification branch from 455e8c0 to 604b2af Compare September 3, 2025 07:19
Take architecture and compiler into account while
grouping tests in `evaluate_test_results`.

Signed-off-by: Jeny Sadadia <[email protected]>
@JenySadadia JenySadadia force-pushed the tree-report-notification branch from d150edd to 1c1e874 Compare September 3, 2025 11:58
Copy link
Collaborator

@MarceloRobert MarceloRobert Sep 3, 2025

Choose a reason for hiding this comment

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

It is working, but I think that the final result is not the best looking. It shows the config for every test, and seems to group tests by architecture even though the first parameter is the config.

IMO we could place the config_name above arch/compiler and add more spaces to group arch/compiler under configs, and then tests under arch/compiler

Current structure:
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

In the example above, it would mean:

Hardware: mt8395-genio-1200-evk
Config: defconfig+lab-setup+kselftest
- Architecture/compiler: arm64/gcc-12
  - boot
    last run: https://d.kernelci.org/test/maestro:68b7e1f7b9811ea53d01b575
    history:  > ✅  > ✅  > ❌  > ✅  >

Config: defconfig+preempt_rt
- Architecture/compiler: arm64/gcc-12
  - rt-tests.cyclicdeadline
    last run: https://d.kernelci.org/test/maestro:68b79c4eb9811ea53d016421
    history:  > ✅  > ⚠️  
            
  - rt-tests.cyclictest
    last run: https://d.kernelci.org/test/maestro:68b79c53b9811ea53d016440
    history:  > ✅  > ⚠️ 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, thanks for the testing. I fixed the summary template.
Tested OK locally. Thanks for helping me with the local setup @MarceloRobert :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @padovan
Does this structure look OK or do you need any changes? #1453 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

How about add one more level of indent

Hardware:
  > Config:
      -  Arch/compiler:

So it reads better.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about add one more level of indent

Hardware:
  > Config:
      -  Arch/compiler:

So it reads better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. Updated it.

Hardware: sun50i-h5-libretech-all-h3-cc
  > Config: defconfig+arm64-chromebook+kselftest
    - Architecture/compiler: arm64/gcc-12
      - kselftest.uevent
      last run: https://d.kernelci.org/test/maestro:68be1e3f3273d911562c70a2
      history:  > ✅  > ❌  > ✅  > ✅  > ✅  
            
      - kselftest.uevent.uevent_uevent_filtering
      last run: https://d.kernelci.org/test/maestro:68be330a3273d911562cd016
      history:  > ✅  > ❌  > ✅  > ✅  > ✅  
            
      - kselftest.uevent.uevent_uevent_filtering_global_uevent_filtering
      last run: https://d.kernelci.org/test/maestro:68be330a3273d911562cd017
      history:  > ✅  > ❌  > ✅  > ✅  > ✅  

@JenySadadia JenySadadia force-pushed the tree-report-notification branch from 90b9e78 to de9719f Compare September 4, 2025 12:00
Copy link
Collaborator

@MarceloRobert MarceloRobert left a comment

Choose a reason for hiding this comment

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

Seems to be working and looking good! I won't merge it, you can wait for Padovan's check and merge it later or you can do so now, you have the permission

Jeny Sadadia added 2 commits September 8, 2025 11:21
Update `render_issues` macro to take into
account `arch/compiler` string while iterating
over issues for displaying checkout summary.

Signed-off-by: Jeny Sadadia <[email protected]>
Update `RegressionData` model to accommodate newly
added `arch/compiler` string.

Signed-off-by: Jeny Sadadia <[email protected]>
@JenySadadia JenySadadia force-pushed the tree-report-notification branch from de9719f to d63a5fc Compare September 8, 2025 05:52
@JenySadadia
Copy link
Contributor Author

Merging this one as all the comments have been addressed.

@JenySadadia JenySadadia merged commit 1003013 into main Sep 8, 2025
7 checks passed
@JenySadadia JenySadadia deleted the tree-report-notification branch September 8, 2025 10:28
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.

3 participants