Mbed-os: FAT filesystem test failed to build after "PR #13549 Make readdir reentrant" merged

[jamesbeyond]

Description of defect

Greentea test STORAGE-FILESYSTEM-FAT-TESTS-TESTS-FAT-FAT_FILESYSTEM
Failed to build after #13549 merged.
and throw error

Build failures:
  * K64F::GCC_ARM::STORAGE-FILESYSTEM-FAT-TESTS-TESTS-FAT-FAT_FILESYSTEM
        Building project fat_filesystem (K64F, GCC_ARM)
        Scan: GCC_ARM
        Scan: fat_filesystem
        Compile [100.0%]: main.cpp
        [Error] main.cpp@141,26: cannot convert 'mbed::Dir*' to 'DIR*' {aka 'DIR_impl*'}

Target(s) affected by this defect ?

Not related to specific targets

Toolchain(s) (name and version) displaying this defect ?

GCC_ARM 9

What version of Mbed-os are you using (tag or sha) ?

latest Master 2f87d59c7f2eed226ff82df72d2724b0ac122177

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

current version

How is this defect reproduced ?

mbed test -m K64F -t GCC_ARM --compile -DMBED_EXTENDED_TESTS -n *fat* -c

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2605

[0xc0170]

@kjbracey-arm Please review

[kjbracey-arm]

Weird type mismatch in that test. Using mbed::Dir with readdir(DIR *).

It's not obvious that that should work, and I don't think it's documented. Can't use mbed::FileHandle with read and write.

I did deliberately tighten that up, as the various castings going on were rather iffy from a C/C++ language compatibility standpoint; wasn't aware there was a test relying on it. (Why didn't it run on the PR?)

As the test is using Dir, I believe it should look like

while (dir.read(&de)) {

readdir is used with the DIR * returned from POSIX opendir. It would be a bigger rewrite to the test to use that, and this isn't a POSIX readdir test

[jamesbeyond]

The direct reason for the test not being run, is because, those tests are considered as HW independent so no need running them on every targets. so the test are not running by default because they are guarded by MBED_EXTENDED_TESTS macro

https://github.com/ARMmbed/mbed-os/blob/master/storage/filesystem/fat/tests/TESTS/fat/fat_filesystem/main.cpp#L29
similarly happens to the events tests:
https://github.com/ARMmbed/mbed-os/blob/master/events/tests/TESTS/events/equeue/main.cpp#L18

I can't recall any other reason for those test being excluded.
I the current owner @evedon happy about it, I can remove the macro, and let tests run by default

[kjbracey-arm]

I don't think we need to switch them to run on every target. Just at least one :) That test was apparently was compiled on no targets during the PR.

I thought the extended tests were done on K64F and maybe a couple of others.

[evedon]

Good suggestion from Kevin: one target is enough.

[LDong-Arm]

Created #13949 exactly as @kjbracey-arm proposed.

@jamesbeyond I've checked that all -DMBED_EXTENDED_TESTS tests pass, once it's merged we can probably add extended tests for one target to CI.