Skip to content
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.

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

🧑‍💻Improve make tests-all-local #27040

Merged

Conversation

sjasonsmith
Copy link
Contributor

@sjasonsmith sjasonsmith commented May 2, 2024

Description

This resolves several issues encountered when running make tests-all-local. These were found when operating inside Windows Subsystem for Linux, but likely impacted other development environments as well:

  1. Tests continued after failing a TEST_TARGET. This made it difficult to notice when a failure occurred. By adding a test for status and an exit(1) call on failure, the test sequence now stops as soon as a failure occurs.

  2. PlatformIO frequently failed to transition from one test target to the next, with errors indicating missing frameworks. This seems to be resolved by adding a 5 second delay when transitioning from one TEST_TARGET to the next.

  3. Test the local Python to ensure pyyaml is available. If not, instruct user how to install it.
    I chose not to install it automatically, since this is the user's default Python and not a virtual environment.

  4. Fix broken file reference, and do not require Python files to be made executable to work.

Requirements

  • Linux-like development environment with make and other Marlin dependencies installed
  • Execute make tests-all-local from the command line

Benefits

Simplified running compilation tests locally to test changes outside of a PR.

@sjasonsmith
Copy link
Contributor Author

@thisiskeithb would you be able to test this on macOS to make sure I haven't broken your workflow while fixing my own?

@sjasonsmith sjasonsmith marked this pull request as draft May 2, 2024 05:13
@sjasonsmith
Copy link
Contributor Author

There are apparently some more changes I need to make. I think I edited some other files that I forgot to check in.

@thisiskeithb
Copy link
Member

thisiskeithb commented May 2, 2024

I've not had good luck with running most of Marlin's scripts, so I usually send them off to GitHub and let CI handled them.

I still have the same Permission denied error when trying to run make tests-single-local / make tests-all-local unless I run chmod +x get_test_targets.py before they'll attempt to run. Could all of these scripts be set to executable by default like we do for files in Marlin/buildroot/tests/? Or somehow automate it? Switching branches / testing out all kind of forks makes this a pain.

Once I make get_test_targets.py executable and run make tests-single-local TEST_TARGET=FYSETC_F6, it has several macros.h warnings, but works:

Passed
All tests completed successfully

Running make tests-all-local, it fails with a new error (same error I get before this PR):

Traceback (most recent call last):
  File "/Marlin/buildroot/share/scripts/get_test_targets.py", line 11, in <module>
    with open(yaml_file) as f:
         ^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '.github/workflows/test-builds.yml'

I documented some of these issues & potential fixes in #26904 (comment), but it would be really handy if there was a set of instructions somewhere that had the list of prerequisites on what all needs to be installed / set to executable or automate it so this process is easier.

Tested on macOS Monterey

@thisiskeithb
Copy link
Member

After your latest commits... Success!

Passed
All tests completed successfully

Tested on macOS Monterey

@sjasonsmith
Copy link
Contributor Author

After your latest commits... Success!

Passed
All tests completed successfully

Tested on macOS Monterey

Thanks for testing it out, and I'm glad we could fix the macOS failure!

@sjasonsmith sjasonsmith marked this pull request as ready for review May 2, 2024 13:36
@sjasonsmith sjasonsmith merged commit 39f53c3 into MarlinFirmware:bugfix-2.1.x May 2, 2024
62 checks passed
RPGFabi pushed a commit to RPGFabi/Marlin that referenced this pull request Jun 15, 2024
* Sleep 5 seconds between platforms

* Inform users if they need to install pyyaml

* Fix old workflow name

* Skip linux_native on Darwin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants