Arm unittest refactor of Add and Conv2D test cases

[AdrianLundell]

This is a showcase of multiple improvements which can be done across all tests:

1. Move pipeline definition from tests to a general (flexible) pipeline class

• Define a default step of stages using e.g. TosaPipelineBI()
• Add custom config or debug stages using helper functions s.a. .change_args(), .add_stage(), .dump_artifact() etc.
• Run the full pipeline using .run()

2. Move towards a pure pytest approach to remove dependencies on unittest and parametrize
3. Separate tests running on FVP from tests not running on FVP rather than configuring this from the command line

• FVP tests are skipped if not installed
• To filter out tests one may instead use pytest markers/name filtering
• This should give a clearer picture of what has been tested

4. Introduces one favored way of marking tests as xfails, in the parameterize decorator

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/7541

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure, 1 Cancelled Job, 1 Pending

As of commit 1bf0d04 with merge base 4796da7 ():

NEW FAILURE - The following job has failed:

• trunk / test-huggingface-transformers (google/gemma-2-2b) / linux-job (gh)
  RuntimeError: Command docker exec -t eca3920b26d0cfbc9def0e38d60a3cd5f27747ea675c250fc502a3140bc648f4 /exec failed with exit code 1

CANCELLED JOB - The following job was cancelled. Please retry:

• trunk / test-pybind-build-macos (cmake) / macos-job (gh)
  ##[error]The operation was canceled.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[AdrianLundell]

@digantdesai Curious to see what you think about this, in particular this removes unittest support in favour of a simpler pytest-only setup + it introduces one extra layer of complexity by wrapping test stages in the new test_pipeline.

[digantdesai]

I love the new approach. It's easier to add tests, cleaner, and reduces code duplication.

However, I have a concern that this might lead to "blind copy-paste" of new tests without thoroughly thinking about the test cases, especially from new users. Over time, we may end up with solid coverage for things that are already covered in the pipeline generator, but leave gaps elsewhere. For example, consider the conv test from your PR. When adding a module with two_conv, if I were to manually add a test case, in edge IR after to_edge, I would check for two convolution nodes to be present. Although it's possible to add a stage in the pipeline object, someone more likely to just do what you did.

Originally, the tester was designed to address code duplication and redundancy, without taking away too much control.

That said, at this point, my concerns might not be too high, so I think we can proceed with this approach. We can evaluate when to use this more vigilantly.

[AdrianLundell]

Big thanks for the through review! I've addressed your comments + some internal ones (mostly removing some unused code), let me know what you think

[facebook-github-bot]

@digantdesai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[digantdesai]

LGTM, bunch of CI issues, not sure how many are related. Please make it green. Let me pull it in internally, see if something goes wrong with parametrize

[AdrianLundell]

Great, I'll look into it when CI is stable again

[facebook-github-bot]

@digantdesai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[AdrianLundell]

None of these CI-failures look related to me

[digantdesai]

rebase please, couldn't import it internally. Thanks.

[AdrianLundell]

@digantdesai Are you able to import it now?

[digantdesai]

Trying again now.

[facebook-github-bot]

@digantdesai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[zingo]

@digantdesai will you handle the merged or should we do it ? Not really sure about best action when PR is "imported". :)