tests: Add options.

[Ouss4]

Description of Change

Add multiple options to the test scripts.

1. Add command line options for flash mode and parts of the FQBN (others or any required argument can be added in the future).
2. Add a way to have multiple FQBNs per test. The FQBNs are stored in a JSON file. The test will be built and run for every FQBN.
3. Option to erase the flash before programming. This is very hacky and shouldn't be here. For one, in the CI, it will always erase the flash. Locally one can omit the -e option. Second, this belongs to pytest-embedded, and should be added there, tests that require to erase the flash will be parameterized something like:

@pytest.mark.parametrize(
  'erase_flash'=[True], indirect=True,
)
def test_hello_world()
   ...

There is a function already that does this but it's not public and only part of the IDF service. This needs to be generic.

These options can be used as:

1. No options at all. This is a compatible behavior with what we already have. In this case a default FQBN will be used.
2. Pass options through the command line, like -fm dio; here the FQBN will be constructed from these options.
3. For tests where multiple FQBNs are required for one chip, the config file can be used. Each element in the fqbn array will have a corresponding build.

Tests scenarios

Locally with the provided scripts.
CI will test the rest.

Related links

#6885
Closes #6884

[Ouss4]

S3 builds are failing with:

Building hello_world with FQBN=espressif:esp32:esp32s3:FlashMode=qio,FlashFreq=80,FlashSize=4M,PartitionScheme=huge_app
Error resolving FQBN: getting build properties for board espressif:esp32:esp32s3: invalid option 'FlashFreq'

Why doesn't S3 have a FlashFreq option? I see in https://github.com/espressif/arduino-esp32/blob/master/boards.txt that it's the only chip without. Is there a reason for that?

(This doesn't fail on the on-push workflow because there we pass a fixed FQBN that does not contain FlashFreq.)

[me-no-dev]

Why doesn't S3 have a FlashFreq option? I see in https://github.com/espressif/arduino-esp32/blob/master/boards.txt that it's the only chip without. Is there a reason for that?

Yes there is a reason and future chips will probably be the same. We used to have way too many options available and too many cases to cover (8 bootloaders + some different libs for each chip). With OPI and 120MHz as options, that would have brought even more bootloaders, etc. So for S3 we go simple. See here

[Ouss4]

So for S3 we go simple. See #6885 (comment)

Simple is better, however what's the plan for older chips? Is there any plans to drop the extra options and combine flash mode and frequency or S3 is gonna stay as a special case for now?

It's simple to just #ifdef S3 the difference now, but it would be better to have something generic.

[me-no-dev]

So for S3 we go simple. See #6885 (comment)

Simple is better, however what's the plan for older chips? Is there any plans to drop the extra options and combine flash mode and frequency or S3 is gonna stay as a special case for now?

Future chips, I hope, will be like S3. For C3 and S2 we can probably go more or less the same, except OPI and 120MHz, but on ESP32 situation is different and I have boards that run on dout, dio and qio (different freqs as well) so for that chip, things must stay as they are

[me-no-dev]

I will suggest to extract the option names also from some argument. We should not be limited by these options, nor we should need to change them if we adjust the options in Arduino's configs. We can have the default FQBNs defined somewhere, if special is needed, then JSON should be provided.

[Ouss4]

I only added the basics that (I thought) will be present in all chips. For further customization one can pass a whole FQBN, this is not very different from passing options with their names. It can save some typing if we extended options to the common parts, for instance for S3 the FQBN will start with espressif:esp32:esp32s3: and then we extend it with ./tests_build.sh -t esp32s3 -a "FlashMode=qio120,PartitionScheme=huge_app, the resulting FQBN will be espressif:esp32:esp32s3:FlashMode=qio120,PartitionScheme=huge_app

[Ouss4]

In this case those flash options are useless and cumbersome, I can just remove them.

[Ouss4]

In this case those flash options are useless and cumbersome, I can just remove them.

I did so.

[github-actions]

Unit Test Results

15 files  15 suites   2m 10s ⏱️
  7 tests   7 ✔️ 0 💤 0 ❌
24 runs  24 ✔️ 0 💤 0 ❌

Results for commit 05c07ee.

♻️ This comment has been updated with latest results.

[VojtechBartoska]

@Ouss4 can this be merged? Thanks!

[Ouss4]

@Ouss4 can this be merged? Thanks!

From my side yes, unless someone else has any concerns or new idea to add.

[VojtechBartoska]

@SuGlider & @P-R-O-C-H-Y Please help with reviews and we can proceed to merging.

[SuGlider]

LGTM

[Ouss4]

Looks like the branch was rebased using the Web UI which added a merge commit. Last force push was just to remove it.

[SuGlider]

Looks like the branch was rebased using the Web UI which added a merge commit. Last force push was just to remove it.

Sorry @Ouss4, my bad. Can you fix it back?

[Ouss4]

Looks like the branch was rebased using the Web UI which added a merge commit. Last force push was just to remove it.

Sorry @Ouss4, my bad. Can you fix it back?

No problem, I already did. :)

[P-R-O-C-H-Y]

Lets merge it :)

[VojtechBartoska]

I promise last question before a merging :)

@Ouss4 There is bunch of commits, do you want to keep a history and maybe clean it a bit or can it be squashed into 1 commit?

[Ouss4]

Usually, I keep separate logical changes in different commits, but the workflow here was to squash merge everything from the UI, so I may not have payed a lot of attention to the history. Feel free to squash merge everything in one commit.