[U-Boot] mmc tests incorrectly implemented

[Stephen Warren]

On 4/10/19 10:23 AM, Marek Vasut wrote:
> On 4/10/19 5:12 PM, Stephen Warren wrote:
> 
> Hi,
> 
> it would be nice if I was CCed on this.

Sorry, I didn't drill down in Jenkins/git data to find out where the
commits came from; I just had a list of commit descriptions and knew
which branch they showed up in,

>> I see that some mmc tests have been added to test/py, but I see problems
>> with them:
>>
>> 1) test_mmc_rescan assumes that each entry in env__mmc_rd_configs is a
>> separate device that can be rescanned. This isn't actually true; entries
>> in that array are intended to drive the mmc read test, and so can point
>> at partitions or specific sector numbers.
> 
> Is that documented somewhere ? I assumed they are separate devices and
> if you need to test multiple partitions, you will have multiple entries
> in this array, one for each device:partition pair.

There is an example in a comment at the top of test_mmc_rd.py, since
that's what is intended to use this data. Admittedly, it doesn't
explicitly spell out that the data array is intended for sole use by
that test, but I assumed it was obvious enough since the array was named
after the test and only used/mentioned in that one tests.

>> Running an mmc rescan test on
>> the entire array results in duplicated tests. A new data array should be
>> created for different tests.
> 
> I don't have such a usecase, but the fix should be easy to implement.
> Can you do that ?

Surely you have a use-case for the new test, or you wouldn't have
implemented it?

I'm afraid I barely have time to keep the test system running. I don't
have time to patch up test failures in most cases. The only option I
have available is to disable all MMC testing on my boards if that's
what's needed to keep the test system going to other tests. Sorry.

>> 2) It relies on test data that was not previously required. In
>> particular, I see new test failures because the "info_device" key is now
>> required. New tests should either (a) use new optional data arrays to
>> configure their operation, or (b) access any new data in an optional
>> way, skipping the test if it's not present, so as not to cause test
>> failures.
> 
> Either that, or update your tests. Can you submit a patch for this ?
> 
>> Also, shouldn't things like new tests be announced to the board
>> maintainers list, so that people can update their test/py configurations
>> to enable new tests if they're appropriate for their platform?
> 
> I don't know , should it ?

Well, why not? How else are people expected to find out about new tests
and know to enable them on their HW if appropriate? I certainly don't
think that expecting people running test systems to read every single
email or commit message to poll for changes to the tests is reasonable.

>> [-] Section: test_mmc_info[emmc-boot0]
>> FAILED:
>> u_boot_console = <u_boot_console_exec_attach.ConsoleExecAttach object at
>> 0x7fafd88f2d10>
>> env__mmc_rd_config = {'count': 1, 'devid': 0, 'fixture_id':
>> 'emmc-boot0', 'is_emmc': True, ...}
>>
>>     @pytest.mark.buildconfigspec('cmd_mmc')
>>     def test_mmc_info(u_boot_console, env__mmc_rd_config):
>>         """Test the "mmc info" command.
>>
>>         Args:
>>             u_boot_console: A U-Boot console connection.
>>             env__mmc_rd_config: The single MMC configuration on which
>>                 to run the test. See the file-level comment above for
>> details
>>                 of the format.
>>
>>         Returns:
>>             Nothing.
>>         """
>>
>>         is_emmc = env__mmc_rd_config['is_emmc']
>>         devid = env__mmc_rd_config['devid']
>>         partid = env__mmc_rd_config.get('partid', 0)
>>>       info_device = env__mmc_rd_config['info_device']
>> E       KeyError: 'info_device'
>>
>> src/u-boot/test/py/tests/test_mmc_rd.py:151: KeyError