tests: drivers: spi: spi_loopback: skip tests if invalid config

[bjarki-andreasen]

The spi_loopback_transceive() helper currently only prints a message if a configuration is invalid, continuing the test case as if it succeeded. This results in the test case using the helper trying to validate the result from a spi transaction that was skipped.

Fix this by explicitly skipping the test using the ztest framework's ztest_test_skip() function, which skips the entire test case.

Before this patch:

===================================================================
START - test_spi_word_size_9
E: Word sizes other than 8 bits are not supported
Spi config invalid for this controller - skip

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/spi/spi_loopback/src/spi.c:628: spi_loopback_test_word_size: (memcmp(compare_data, rx_buffer, buffer_size) is true)
9-bit word buffer contents are different
 FAIL - test_spi_word_size_9 in 262.779 seconds
===================================================================

After this patch

START - test_spi_word_size_9
E: Word sizes other than 8 bits are not supported
Spi config invalid for this controller
 SKIP - test_spi_word_size_9 in 0.009 seconds

fixes: #90318

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[decsny]

So, when I made this PR originally for this tests, I had done skips instead, but there was some kind of Ztest bug that was making a skip cause the test to fail so we decided not to use the skips. I don't know if it got fixed, FYI @djiatsaf-st and @yperess

[bjarki-andreasen]

So, when I made this PR originally for this tests, I had done skips instead, but there was some kind of Ztest bug that was making a skip cause the test to fail so we decided not to use the skips. I don't know if it got fixed, FYI @djiatsaf-st and @yperess

ok, will wait to hear back, but ztest_test_skip() is used widely in zephyr, it would have to be a platform bug of some sort I would imagine then :)

[decsny]

So, when I made this PR originally for this tests, I had done skips instead, but there was some kind of Ztest bug that was making a skip cause the test to fail so we decided not to use the skips. I don't know if it got fixed, FYI @djiatsaf-st and @yperess

ok, will wait to hear back, but ztest_test_skip() is used widely in zephyr, it would have to be a platform bug of some sort I would imagine then :)

no , it wasn't a platform bug, it was something with ztest, the test case would skip on the ST parts, and then the whole test suite would fail despite no individual test case failing, so we decided to just make the test pass instead of skip so the PR could merge

[JarmouniA]

ok, will wait to hear back, but ztest_test_skip() is used widely in zephyr, it would have to be a platform bug of some sort I would imagine then :)

@bjarki-andreasen #86611

[bjarki-andreasen]

@JarmouniA so, seems it is fixed now? We can use ztest_test_skip()?

[JarmouniA]

@JarmouniA so, seems it is fixed now? We can use ztest_test_skip()?

It appears so, @djiatsaf-st can confirm.

[djiatsaf-st]

@JarmouniA so, seems it is fixed now? We can use ztest_test_skip()?

It appears so, @djiatsaf-st can confirm.

Sorry, I missed this ping.

Yes, I can confirm that ztest_test_skip() will work for the appropriate test.

After making some tests on PR #87838 for some STM32 boards, I have this output as an example:

START - test_spi_word_size_24 
Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/spi/spi_loopback/src/spi.c:204: spi_loopback_transceive: (ret is non-zero) 
SPI transceive failed, code -14 
FAIL - test_spi_word_size_24 in 0.015 seconds 
=================================================================== 
START - test_spi_word_size_32 

Spi config invalid for this controller 

SKIP - test_spi_word_size_32 in 0.004 seconds 
=================================================================== 
START - test_spi_word_size_7 
Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/spi/spi_loopback/src/spi.c:204: spi_loopback_transceive: (ret is non-zero) 

SPI transceive failed, code -14 

FAIL - test_spi_word_size_7 in 0.015 seconds 
=================================================================== 
START - test_spi_word_size_9 
Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/spi/spi_loopback/src/spi.c:204: spi_loopback_transceive: (ret is non-zero) 
SPI transceive failed, code -14 

FAIL - test_spi_word_size_9 in 0.015 seconds 
=================================================================== 

I think to be more complete, we should update the condition to add EFAULT (error number 14) to handle the failed test on the drivers.spi.stm32_spi_dma.loopback scenario:

 if (ret == -EINVAL || ret == -ENOTSUP || ret == -EFAULT) { 
 ...
 }

[bjarki-andreasen]

@djiatsaf-st

-EFAULT would be an actual fault though? we are only skipping tests for which the hardware can not support the test case, see

zephyr/include/zephyr/drivers/spi.h

Lines 897 to 901 in 66eb61a

	* @retval 0 If successful in master mode.
	* @retval -ENOTSUP means some part of the spi config is not supported either by the
	* device hardware or the driver software.
	* @retval -EINVAL means that some parameter of the spi_config is invalid.
	* @retval -errno Negative errno code on failure.

so if -EFAULT is used to describe an invalid config, the driver should be updated to return -ENOTSUP or -EINVAL :)

[djiatsaf-st]

@djiatsaf-st

-EFAULT would be an actual fault though? we are only skipping tests for which the hardware can not support the test case, see

zephyr/include/zephyr/drivers/spi.h

Lines 897 to 901 in 66eb61a

	* @retval 0 If successful in master mode.
	* @retval -ENOTSUP means some part of the spi config is not supported either by the
	* device hardware or the driver software.
	* @retval -EINVAL means that some parameter of the spi_config is invalid.
	* @retval -errno Negative errno code on failure.

so if -EFAULT is used to describe an invalid config, the driver should be updated to return -ENOTSUP or -EINVAL :)

Thank you for your reply. I will make the necessary changes in another PR to ensure things work as expected without -EFAULT.

That said, I'm okay with changes on this PR 👍

[bjarki-andreasen]

ping @tbursztyka :)