Skip to content

Remove tinytest framework, and --write-exp and --list-tests options from run-tests.py #15788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Sep 19, 2024
Merged

Remove tinytest framework, and --write-exp and --list-tests options from run-tests.py #15788

merged 2 commits into from
Sep 19, 2024

Conversation

dpgeorge
Copy link
Member

@dpgeorge dpgeorge commented Sep 4, 2024

Summary

With the recent qemu (#15624 and #15743) and zephyr (#15678) changes to how their tests are run, two things became unused:

  • the tinytest framework, which embedded a set of tests and their expected output within firmware, so these tests could be run stand-alone
  • the --write-exp and --list-tests options to tests/run-tests.py, which were needed primarily to generated the expected test output for tinytest (also the associated tests/run-tests-exp.py/.sh scripts are unused)

This PR removes both of these things.

Rationale

Removing tinytest removes now-unused code and reduces a maintenance burden.

Removing --write-exp and --list-tests options helps to simplify the rather complex logic in run-tests.py. In particular this opens the door to make it so run-tests.py can automatically detect the target/platform that it's running on (that's for a follow-up PR).

Testing

All code removed here is unused. The existing tests and CI should not change.

Trade-offs and Alternatives

We could leave this functionality here, in case it's useful for people to use in their own project. But IMO that's a burden on this project that we don't need. And we'd need to add code to test these now-unused components, otherwise they will eventually break as other things change around them.

@dpgeorge
Copy link
Member Author

dpgeorge commented Sep 4, 2024

@stinos @dlech I would appreciate your review/comments on this PR. You are the most likely to be using these now-obsolete features, and if you're not then it's unlikely anyone else is 😄

I'd rather not keep these unused parts, but if you have a strong reason to keep them please let me know.

@dpgeorge dpgeorge added the tests Relates to tests/ directory in source label Sep 4, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 4, 2024

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

@codecov Codecov
Copy link

codecov bot commented Sep 4, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.44%. Comparing base (5d8878b) to head (1be38e8).
Report is 2 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #15788   +/-   ##
=======================================
  Coverage   98.44%   98.44%           
=======================================
  Files         163      163           
  Lines       21296    21296           
=======================================
  Hits        20964    20964           
  Misses        332      332

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stinos
Copy link
Contributor

stinos commented Sep 4, 2024

I'm not actively using --list-tests and don't think I ever used --write-exp so as far as I'm concerned these can be removed.

@dlech
Copy link
Contributor

dlech commented Sep 4, 2024

Thanks for the heads-up. We are using lib/tinytest in our Pybricks tree. But it will be no problem to just copy that directory to our own tree.

@dpgeorge
Copy link
Member Author

dpgeorge commented Sep 5, 2024

We are using lib/tinytest in our Pybricks tree

Ah, that's interesting. Do you find it easier/necessary to use tinytest over just running the tests through the serial port?

@dlech
Copy link
Contributor

dlech commented Sep 5, 2024

We actually just use it for unit testing C code, not MicroPython stuff.

@dpgeorge
Copy link
Member Author

dpgeorge commented Sep 5, 2024

We actually just use it for unit testing C code

Right. So you don't use upytest_execute_test, or probably any of those helper functions.

Then it is better for you to just maintain a copy of the tinytest library. Not really much point in us having it here.

@agatti agatti mentioned this pull request Sep 6, 2024
Closed
@dpgeorge dpgeorge requested a review from projectgus September 11, 2024 03:42
projectgus
projectgus approved these changes Sep 11, 2024
View reviewed changes
Copy link
Contributor

@projectgus projectgus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always a good day when you can delete code without losing functionality!

(And in general I think this set of changes to run all tests through the same codepaths is really beneficial!)

@dpgeorge
all: Remove tinytest component.
067ef81 
With the recent qemu (d9a0fdd and
0426934) and zephyr
(05cad7b) changes to how their tests are
run, two things became unused:

- The tinytest framework, which embedded a set of tests and their expected
  output within firmware, so these tests could be run stand-alone.

- The `--write-exp` and `--list-tests` options to `tests/run-tests.py`,
  which were needed primarily to generated the expected test output for
  tinytest (also the associated `tests/run-tests-exp.py/.sh` scripts are
  now unused).

This commit removes the tinytest component and all its helper code.  This
eliminates a maintenance burden.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge
tests/run-tests.py: Remove --write-exp and --list-tests options.
1be38e8 
Removing the now-unused (see previous commit for details) `--write-exp` and
`--list-tests` options helps to simplify the rather complex logic in
`run-tests.py`.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge merged commit 1be38e8 into micropython:master Sep 19, 2024
63 of 64 checks passed
@dpgeorge dpgeorge deleted the tinytest-removal branch September 19, 2024 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@projectgus projectgus projectgus approved these changes

Assignees
No one assigned
Labels
tests Relates to tests/ directory in source
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dpgeorge @stinos @dlech @projectgus