Run the test suite as a GitHub Action #208

EmperorArthur · 2024-08-04T05:32:37Z

No description provided.

Required on headless systems.

pimlie · 2024-08-08T10:33:45Z

tests/openscad_runner.py

    scad_file_path: Path
    openscad_binary_path: Path
    image_folder_base: Path
    parameters: Optional[dict]
    '''If set, a temporary parameter file is created, and used with these variables'''

-    WINDOWS_DEFAULT_PATH = 'C:\\Program Files\\OpenSCAD\\openscad.exe'
+    LINUX_DEFAULT_PATH = Path('/bin/openscad')


This could be just openscad, no need to add the /bin/ prefix as Linux resolves the path by whatever you put in env.PATH

Does it in cases where you're not running through a shell?

Edit: I also don't think the automatic expansion works when doing the exists() check.

I am a fan of the absolute path as it won't depend on the users environment. If something is by default on Linux it does not mean we need to depend on it when it is not necessary. Defaults van change, users can do weird shit. We already have a big dependency on openscad. If they change the install location, it fails everywhere and we can change it.

pimlie · 2024-08-08T10:41:45Z

.github/workflows/ci.yml

+    - name: Install OpenSCAD
+      run: |
+        sudo apt-get update
+        sudo apt-get install openscad


This most likely installs the v2021 version, which is quite old. It might be beneficial to either install/use a nightly (appimage) version or even better use openscad's docker container for these tests.

I do agree it is very old but it is the latest stable afaik.

Maybe it is possible to use a fixed "nightly" version? I hate the ci/cd breaking due to the nightly being updated and unstable. I also don't like an openscad binary in this repo.

tests/openscad_runner.py

pimlie · 2024-08-08T11:45:37Z

FWIW, I had also been working on adding a github action. You can see my WIP branch for that here: https://github.com/kennetek/gridfinity-rebuilt-openscad/compare/main...pimlie:test-use-bash-github-yaml?expand=1 It's still a WIP as I was testing the workflow with act but ran into an issue with the work folder.

Not sure if it's worth continuing that branch, but to explain my choices:

I used bash and not python so we could run tests directly using the openscad docker image once built-in EGL support lands (see remark on https://github.com/openscad/docker-openscad under 'Rendering a PNG).
The Python tests contain a lot of repetition, but due to way the tests are setup using the parameterized .scad files, we basically only ever need to change the scad parameters and maybe camera angle. Therefore I opted to use yaml files to configure the tests as that has a much lower threshold for adding new tests
I was planning to adding support for image comparison similar to openscad does

But I will hold off on working on my changes for now as to now 'compete' with the work from this pr.

EmperorArthur · 2024-08-09T03:49:30Z

@pimlie I suspect it's partly also a methodological approach.

I chose Python's unittest because it's a simple test framework that's easy to install and run. I'm not opposed to other test frameworks at all, but feel that trying to homebrew a testing framework just to avoid a dependency causes far too many problems.
Do your tests run under Windows? It is very common for git bash to be installed, but it's not required for someone to use git.
You might notice the Python tests can also use scad parameter files for some tests. The difference is instead of one file per test, the parameter being tested is modified.
This is explicitly doing unit tests at the library level. Hence all the pictures of hole negatives.
Image comparison would be nice, but I wonder if we could use csg export. No major feelings either way.

Future Work

Realistically, the hole tests should be changed to do a matrix setup instead of brute force copy/paste,
I use Python's unittest framework not because it's the best test framework but because it's part of Python, and that is the easiest cross platform tool I am familiar with.

pimlie · 2024-08-09T09:41:06Z

@EmperorArthur

RE: Do your tests run under Windows? -> They should under WSL. Native Windows support itself for tests should not be a strict requirement imho, too many possible caveats/limitations. The main env (and thus first priority) that will run the tests should be the github workflow which normally runs directly under Linux, if we can manage to run the tests under Windows too then that could be a nice-to-have but requiring Windows test support could limit the project.
RE: You might notice the Python tests can also use scad parameter files for some tests -> I did indeed and thought that having a default parameter file and then overwrite individual parameters on the cli was an excellent choice by you for the tests! Hence why I tried to copy that behaviour, I just used yaml to make that 'overwrite' behaviour super clear and simple to do :)

Btw, I looked at quite a bit of other SCAD projects and I didn't find any other projects yet that have fully automated OpenSCAD tests. Have you found any? Cause it would also be nice if we wouldn't have to (re-)invent the wheel for that...

Anyway, don't let my remarks/suggestions keep you from implementing the tests the way you feel is best :)

ntindle · 2024-10-09T03:37:41Z

Just checking into what’s blocking this. Imo the comments are addressed and I’m always in favor of running tests in CI (even in a decreased capacity). Time is cheaper spent in CI than debugging after a break

EmperorArthur · 2024-10-13T21:36:28Z

Just checking into what’s blocking this.

Merging requires at least one approving reviewer. Which has not happened. There are also some merge conflicts I need to address, and I have left it in draft status for the moment.

Right now, I'm focusing on fixing some bugs in the code itself, and getting CI/CD working again.

ntindle · 2024-10-13T21:45:17Z

Gotcha, no pressure just was trying to see if there was a place to help if I had free time

EmperorArthur marked this pull request as draft August 4, 2024 05:35

EmperorArthur force-pushed the ci branch 3 times, most recently from 3fbdfa8 to 207e9dc Compare August 4, 2024 05:51

Run the test suite as a GitHub Action

5585e7f

EmperorArthur force-pushed the ci branch from 207e9dc to 5585e7f Compare August 4, 2024 05:53

First attempt at making the tests Linux compatible

904b8e9

EmperorArthur force-pushed the ci branch from e28da50 to 4e7044b Compare August 4, 2024 06:34

CI: Print some information about the OpenSCAD version used

ea976db

EmperorArthur force-pushed the ci branch from 4e7044b to ea976db Compare August 4, 2024 06:38

EmperorArthur added 2 commits August 4, 2024 02:58

CI: Disable flags which require a beta version of OpenSCAD

df9b7e5

ci: Fully render the geometry when creating images.

49718cc

Required on headless systems.

EmperorArthur force-pushed the ci branch from ac779c7 to 2474b04 Compare August 4, 2024 07:14

ci: another pass at getting OpenSCAD working

8414ffb

EmperorArthur force-pushed the ci branch from 2474b04 to 8414ffb Compare August 4, 2024 07:17

EmperorArthur added 2 commits August 4, 2024 03:22

ci: Ensure image directories exist

42970e7

ci: Upload generated images as artifacts

d59adee

EmperorArthur force-pushed the ci branch from 8d44a8d to d59adee Compare August 4, 2024 07:31

pimlie reviewed Aug 8, 2024

View reviewed changes

Ruudjhuu requested changes Aug 8, 2024

View reviewed changes

tests/openscad_runner.py Show resolved Hide resolved

tests/openscad_runner.py Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run the test suite as a GitHub Action #208

Run the test suite as a GitHub Action #208

EmperorArthur commented Aug 4, 2024

pimlie Aug 8, 2024

EmperorArthur Aug 9, 2024 •

edited

Loading

Ruudjhuu Sep 10, 2024

pimlie Aug 8, 2024 •

edited

Loading

Ruudjhuu Aug 8, 2024 •

edited

Loading

pimlie commented Aug 8, 2024 •

edited

Loading

EmperorArthur commented Aug 9, 2024

pimlie commented Aug 9, 2024

ntindle commented Oct 9, 2024

EmperorArthur commented Oct 13, 2024

ntindle commented Oct 13, 2024

Run the test suite as a GitHub Action #208

Are you sure you want to change the base?

Run the test suite as a GitHub Action #208

Conversation

EmperorArthur commented Aug 4, 2024

pimlie Aug 8, 2024

Choose a reason for hiding this comment

EmperorArthur Aug 9, 2024 • edited Loading

Choose a reason for hiding this comment

Ruudjhuu Sep 10, 2024

Choose a reason for hiding this comment

pimlie Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

Ruudjhuu Aug 8, 2024 • edited Loading

Choose a reason for hiding this comment

pimlie commented Aug 8, 2024 • edited Loading

EmperorArthur commented Aug 9, 2024

Future Work

pimlie commented Aug 9, 2024

ntindle commented Oct 9, 2024

EmperorArthur commented Oct 13, 2024

ntindle commented Oct 13, 2024

EmperorArthur Aug 9, 2024 •

edited

Loading

pimlie Aug 8, 2024 •

edited

Loading

Ruudjhuu Aug 8, 2024 •

edited

Loading

pimlie commented Aug 8, 2024 •

edited

Loading