[microTVM] Remove Arduino aot code

[guberti]

PR #8758 changed the way AOT is handled in microTVM, which caused my Arduino PR to stop working. I should have been caught by the unit tests since it caused Arduino project compilation to stop working, but because I called subprocess.run in microtvm_api_server.py without passing check=True, the tests still pass.

This PR fixes the Arduino implementation of microTVM, changes the unit tests for Arduino to use check=True, and adds a regression test to make sure the unit tests will catch compilation failures.

@mehrdadh@areusch

[mehrdadh]

#8857
You guys need to coordinate on this.

[guberti]

I've rebased the PR on top of main now that #8857 is merged. This PR is ready for review.

[areusch]

@guberti thanks for this--a couple questions about your test fixtures

[guberti]

@guberti thanks for this--a couple questions about your test fixtures

The more I think about it, the more the save_generated_project_state fixture seems like a gross hack. It can create really weird bugs where microtvm_api_server.py is still running, but its source file has been repeatedly deleted and restored.

Even more so, the idea of having these saved project tests doesn't really fit with the test's "goal" - of simulating a user workflow of building and deploying a model on device.

IMO, the cleanest solution is to build a whole new test, test_arduino_error_detection, and move them there. I've done that with 3a8ea4c - take a look and LMK if you agree @areusch.

[Mousius]

Thanks for following up on this @guberti, apologies I didn't pick this up in my original PR - I appreciate your diligence 😸 I just have a few suggestions here.

[Mousius]

Given these fixtures are only used as part of the tests in this folder, and conftest is meant for auto-registration or pytest configuration rather than to hold helpers and utils, could these be either moved fully into conftest as fixtures or the underlying function moved into a more suitably named file?

[guberti]

The scope fixture of workspace_dir varies from test to test (for example, test_arduino_workflowhas it at module scope), and I suspect tests with more complexworkspace_dirsetups will be added in the future. For that reason, I'd prefer to defineworkspace_dirindividually in each test, but use the helper functionmake_workspace_dir` to avoid code reuse. I've also added a comment to each of these functions to clarify it.

That said, if you feel strongly I'm also open to other solutions, like defining multiple kinds of workspace dirs in conftest (e.g. a module_scope_workspace_dir, per_function_workspace_dir, etc.).

[Mousius]

Argh, I feel like I could've been clearer here, my apologies. My suggestion is not to have general functions in conftest.py but rather in a different file such as a workspace.py to include from the test. This means only pytest-specific things end up in conftest.py, I have no issue with fixtures local to the test files which then use those functions.

[guberti]

Ah, that makes more sense. I like the idea of moving non-pytest code into a workspace.py, but I think that might be OOS for this PR. Definitely worth addressing in a follow up though!

[Mousius]

Agreed, it'd be good to have the move isolated in another PR 👍

[areusch]

i agree with @Mousius sentiment--let's place test_utils in a file called test_utils or similar. can be a follow-up PR.

[guberti]

@Mousius Your comments should be addressed by bb37393 - take a look when you have a sec!

[Mousius]

Apologies @guberti, I was a bit unclear in my initial response and I've introduced more changes, feel free to reach out if you need more clarification 😸

[guberti]

Apologies @guberti, I was a bit unclear in my initial response and I've introduced more changes, feel free to reach out if you need more clarification

Oops, it was really late when I made those previous changes :). Should be fixed now @Mousius!

[Mousius]

Looks good to me, removes the AOT code and then some 😸

[leandron]

LGTM