tests: drivers: build_all: sensor: do not run build-only testsuite

[cfriedt]

A recent modification to the build_all/sensor testsuite changed build-only to false in testcase.yaml, which is causing CI to go bonkers.

https://bit.ly/3rSe0Te

Please do not run build-only testsuites.

Fixes #60958

[tristan-google]

Hi @cfriedt et. al., just wanted to follow up on this. I submitted the change that added the sensors.generic_test testcase and made it build+run recently in PR #60394. What this test does is described in that PR's description but the rationale for placing it here was to take advantage of the sensor build_all test's massive devicetree of sensors. It is indented to run but I don't blame you for reverting that to get CI back on track quickly. If there is a more appropriate place to locate this I'm happy to discuss.

On the topic of what went wrong in CI based on the log, a sensor (MAX17262) got stuck in a loop within its init function and spammed the log every 10ms, attempting to make a transaction on the I2C bus. This of course fails since there is neither an emulator nor phsyical device attached, but that shouldn't matter as drivers are expected to fail gracefully if they cannot initialize and simply remain not ready. Taking a look at this driver, I found a bug where the return code from the I2C read/write function calls are not error-checked. So if the read fails the tmp variable is never loaded with an legit value leaving it uninitialized. This never happened in my dozens of local test runs, but apparently in this CI run the right bit happened to be set to get stuck in that polling loop and it held everything up. I'm going to upload a PR for this driver to add error checks to prevent this. I do think there is value in trying to initialize all of these drivers in addition to building them. This would be the second driver bug I've caught through this test (the first surfaced during development and was fixed in #60119)

[cfriedt]

Hi @cfriedt et. al., just wanted to follow up on this. I submitted the change that added the sensors.generic_test testcase and made it build+run recently in PR #60394. What this test does is described in that PR's description but the rationale for placing it here was to take advantage of the sensor build_all test's massive devicetree of sensors. It is indented to run but I don't blame you for reverting that to get CI back on track quickly. If there is a more appropriate place to locate this I'm happy to discuss.

Hi @tristan-google - I think the change you made makes total sense, so just wanted to get that out there. I try to never critique the author but try to focus on critiquing code. In this case, there wasn't even anything wrong with the code per se. It just happened to make CI go kind of off the rails.

On the topic of what went wrong in CI based on the log, a sensor (MAX17262) got stuck in a loop within its init function and spammed the log every 10ms, attempting to make a transaction on the I2C bus. This of course fails since there is neither an emulator nor phsyical device attached, but that shouldn't matter as drivers are expected to fail gracefully if they cannot initialize and simply remain not ready.

100% agree - I didn't go for a deep dive here, but thanks for looking into the root cause.

Taking a look at this driver, I found a bug where the return code from the I2C read/write function calls are not error-checked. So if the read fails the tmp variable is never loaded with an legit value leaving it uninitialized. This never happened in my dozens of local test runs, but apparently in this CI run the right bit happened to be set to get stuck in that polling loop and it held everything up. I'm going to upload a PR for this driver to add error checks to prevent this. I do think there is value in trying to initialize all of these drivers in addition to building them. This would be the second driver bug I've caught through this test (the first surfaced during development and was fixed in #60119)

Awesome - as long as @MaureenHelm is OK with it, I'm ok with that testcase.yaml permutation being run in CI too once the driver is fixed.

Thanks for following up, and sorry for not seeing your message until today, but GH notifications are a bit like drinking from the firehose for maintainers.

[tristan-google]

Thanks Chris. The bug fix for that driver is now merged in #61016. Looking forward to hearing from Maureen.