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

Enable ALSA selftests for AT91SAM9G20-EK #1627

Closed
wants to merge 2 commits into from

Conversation

broonie
Copy link
Member

@broonie broonie commented Jan 10, 2023

This series works around the lack of skipgen support for older ARM systems in the kselftest template then uses that to enable running the ALSA selftests on AT91SAM9G20-EK.

This isn't the most elegant thing ever but given the generally sparing use of skiplists at all and the level of effort required to either port Go or rewrite skipgen it seems like the improvement in test coverage is more than worth the fairly small and non-invasive changes.

@broonie
Copy link
Member Author

broonie commented Jan 11, 2023

This isn't getting tested on staging, we don't build multi_v5_defconfig for v5, only at91_dt_defconfig, and kselftest is set up to only run on multi_v5_defconfig.

@broonie
Copy link
Member Author

broonie commented Jan 11, 2023

Pull request for multi_v5_defconfig in staging kernelci/kernelci-deploy#47

@nuclearcat
Copy link
Member

It looks like at91 booted on baseline-nfs, but didnt appear in alsa tests:
https://staging.kernelci.org/test/job/kernelci/branch/staging-mainline/kernel/staging-mainline-20230120.0/
(and it fails to boot properly in -next )

@broonie
Copy link
Member Author

broonie commented Jan 23, 2023

The -next boot in production looks fine, there are some entries left in deferred-probe-empty (for which I have patches in the list upstream) but it's getting up to the login prompt (baseline.login is passing): https://staging.kernelci.org/test/plan/id/63ca5b3fa28854c8600c5306/ . The ALSA jobs are being generated but failing because the rootfs image isn't there:

http://storage.kernelci.org/images/rootfs/debian/bullseye-kselftest/20230120.0/armel/initrd.cpio.gz' (404)", "Resource unavailable at 'http://storage.kernelci.org/images/rootfs/debian/bullseye-kselftest/20230120.0/armel/full.rootfs.tar.xz

which is a bit of a recursive problem with the version being bumped and invalidating config here - not sure what the best way to handle that is, the usual bodge to use separate staging images feels awfully disruptive for adding a new architecture to kselftest.

@mgalka
Copy link

mgalka commented Jan 24, 2023

I guess it'd be good to have the rootfs config in a separate PR, then we could merge it and have the ALSA tests run on staging. Rootfs images are updated using changes from the main branch every Friday. Staging and prod jobs use the same set of rootfs images by default.

Having said that, I guess if you can push the rootfs definition changes as a separate PR I could run a rootfs build job and we'll have an armel rootfs available for staging jobs. It shouldn't interfere with anything running on prod atm and building the rootfs for armel seem to be working fine https://storage.staging.kernelci.org/images/rootfs/debian/bullseye-kselftest/20230124.0/armel/

@gctucker
Copy link
Contributor

Why do we need to split the PR? The staging.kernelci.org branch can be used to build a rootfs image on staging, and the test can be run too.

@mgalka
Copy link

mgalka commented Jan 24, 2023

Why do we need to split the PR? The staging.kernelci.org branch can be used to build a rootfs image on staging, and the test can be run too.

We can do it on staging but, I guess it'd require staging specific modification of rootfs-images.yaml something like the "bodge" commit done here 0f34c3a
which is not great, but will let us test staging version of rootfs images. So, probably it's better to do it this way.

I guess, I can built bullseye-kselftest images on staging and add a patch that can be applied by pending.py script, to avoid polluting GH with unnecessary commits/PRs.

@mgalka mgalka self-assigned this Jan 25, 2023
@mgalka
Copy link

mgalka commented Jan 25, 2023

I built bullseye-kselftest images on staging and put a patch to use them in the jobs. Let's wait for the results

@gctucker
Copy link
Contributor

I don't think we need this kind of bodge, here's a much cleaner way of doing it with a draft PR:
#1386

So the steps would be:

  • build a rootfs image (or several) using the staging.kernelci.org branch on staging Jenkins
  • create a draft PR to use that image using the staging rootfs types
  • wait for staging results
  • when all results look OK, close the draft PR and merge the actual PR
  • next production update will pick up the new rootfs image (if it's a new type altogether then it needs to be added by hand in YAML on top of the generated one)

@gctucker
Copy link
Contributor

Does this make sense? The advantage is that if something is broken in the rootfs changes, it can be tested before merging the PR.

@broonie
Copy link
Member Author

broonie commented Jan 25, 2023

The bodge commit that @mgalka linked to above was part of a draft PR used in the flow outlined above.

The main issue is that it feels excessively invasive to test something with a widely used rootfs image if you're only trying to cover a single architecture since you have to do all the architectures, and will potentially disrupt a large bunch of other testing while it's happening. It doesn't look like there's a sensible way to just do focused testing for adding the new architecture, and the whole flow doesn't exactly play nice when there's multiple changes in flight for images.

@mgalka
Copy link

mgalka commented Jan 25, 2023

I've created a PR (#1674) as @gctucker suggested. At the moment it can't be applied to staging. I think it may be because #1658 is touching the same file.

Any suggestions on how I should proceed on this one?

@broonie broonie force-pushed the at91sam9g20ek-audio branch from 5dfefd8 to 1942c25 Compare February 21, 2023 00:40
@broonie
Copy link
Member Author

broonie commented Feb 21, 2023

I forgot about @mgalka's pending pull request here and just built my own one last night also sent my own which also isn't taking.

Is it perhaps more trouble than it's worth to try to get this verified in staging? It's a test for a subsystem I maintain running on a board that's only in my lab, any breakage is pretty much going to be rolling straight at me. I've verified this with my own rootfs builds so I'm fairly confident it'll work.

@broonie
Copy link
Member Author

broonie commented Mar 21, 2023

Skip for now due to the ALSA tests being moved to a separate rootfs and trying to push through the basic rootfs for kselftests.

@broonie broonie added the staging-skip Don't test automatically on staging.kernelci.org label Mar 21, 2023
@gctucker
Copy link
Contributor

gctucker commented Apr 6, 2023

@nfraprado @broonie Is the new rootfs image for kselftest-alsa available and working now? If so we should be able to remove the staging-skip label.

@broonie broonie force-pushed the at91sam9g20ek-audio branch from 1942c25 to 22bbedf Compare April 6, 2023 12:45
@gctucker
Copy link
Contributor

gctucker commented Apr 6, 2023

(...and rebase once the skip label has been removed).

@broonie broonie removed the staging-skip Don't test automatically on staging.kernelci.org label Apr 12, 2023
broonie added 2 commits April 12, 2023 18:44
Allow the templates to make use of the architecture variant by passing it
through as a paramenter arch_variant.

Signed-off-by: Mark Brown <[email protected]>
This system has audio hardware, run the ALSA kselfests on it to verify that
it is functioning as expected.

Signed-off-by: Mark Brown <[email protected]>
@broonie broonie force-pushed the at91sam9g20ek-audio branch from 22bbedf to 45ad347 Compare April 12, 2023 17:45
@gctucker
Copy link
Contributor

OK it's not fingind the rootfs image now:
https://lava.sirena.org.uk/scheduler/job/322106

Resource unavailable at 'http://storage.kernelci.org/images/rootfs/debian/bookworm-kselftest/20230407.0/armel/initrd.cpio.gz' (404)

For some reason there's no armel, guess that would need to be added:
https://storage.kernelci.org/images/rootfs/debian/bookworm-kselftest/20230407.0/

@nfraprado
Copy link
Contributor

There was a PR from Mark for that already: #1831

@gctucker
Copy link
Contributor

gctucker commented Jul 5, 2023

Adding skip label because of #1994

@gctucker gctucker added the staging-skip Don't test automatically on staging.kernelci.org label Jul 5, 2023
@nuclearcat
Copy link
Member

Closing, as we are deprecating the legacy system. If this is still an issue or relevant, please re-open it under the Maestro system. Thank you for your contribution, and sorry for the inconvenience.

@nuclearcat nuclearcat closed this Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
staging-skip Don't test automatically on staging.kernelci.org
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants