Add FreeCAD CI regression test suite - #50
snailcatcher wants to merge 3 commits into
Conversation
|
Hi @Rahix, first and foremosgt I'm not a FreeCad Developer, I'm only using it myself to build stuff, but I am a Software developer with AI experience. Since there are many plugins out there and FreeCad has kind of good documentation, it was not very hard to ask codex to do some research and build up a test suite for regression testing (I thought this was your main goal, if not let me know). Since you had done a good job with splitting the functions to separate pieces it was quite easy to implement and codex also found a bug along the way. I had run the test myself in my own fork repo and locally against FreeCad 1.1.1. Please have a look into this PR, I hope it helps you, because I like your plugin ;) Sincerely @snailcatcher |
|
Hi, thanks for sending the PR. I'm happy to hear that you like FusedFilamentDesign! I fully believe that you send this PR in your best intentions to help the project and I do appreciate the gesture. However, on a technical level, I have to tell you that this work is not useful and some in the open-source community would consider it quite rude to send these sort of fully AI-generated pull requests. I will try to give you some perspective as to why that is. The most important thing you have to understand is how precious the time of open-source project maintainers is. Is is our job to keep the project on path for the long run. Since most do this in their free time, they are very limited in how much effort they have available. So it is of utmost importance to keep the codebase "maintainable". That means that it stays very easy to do the necessary work like fixing bugs, updating outdated things, introducing new features, etc. Project maintainers do this by very deliberatly choosing software architecture and design decisions, keeping unnecessary complexity at an absolute minimum, and not accumulating tech debt if at all possible. A consequence is also that it may be better to not include a feature at all over accepting a half-baked solution. The second I accept your PR, I become "responsible" for all eternity to keep the code maintained. I can reach out to you in the future if things break, but the reality is, I will stay mostly alone with having to fix things. And the maintenance is not trivial: The software world moves fast; things stop working, best practices change. And as the codebase itself evolves, this can also necessitate changes to other parts (like a testsuite). Cut and dry, I should just reject a PR that is not up to par, to protect the project. But of course, as a maintainer, I am also interested in moving the project forward. So the unspoken contract is that I put in effort to help you improve your code such that I get to include it later. And maybe even win you as a repeat contributer in the long run. With (fully) AI-generated work, the equation changes. AI generates lots of code effortlessly and I need to dig through all of it to verify it. Even worse, AI generates code that looks very clean and plausible on the surface but usually hides critical architectural mistakes in depth. Figuring these out is a lot more effort still. And the return of working through this is minimal - in the end, it is usually less effort to sit down and reimplement the feature entirely on my own. In fact, I did exactly that now: I took the time to build #51 which is how a regression tests suite should look like in the context of this project and relevant maintainability considerations. And there is a second reason I had to do this now: By putting your AI-PR out there, you put pressure on a project to include the feature because from the outside, it now looks like "the code for it is there". With a testsuite this doesn't matter too much, but with user-facing features, it can grow to be a real problem, as users do not concern with maintainability much and thus have little understanding for pushing back. I will not go into the technical reasons why your PR is unfit, since you mentioned you aren't really familiar with any of this anyway. If you care, I am happy to explain. If you really want to help the project (maybe even in regards to test coverage), I can give you a run down of how #51 works, then you can contribute more tests for the other ffDesign commands that I have not covered yet. The bug report hidden in this PR is correct, there is a regression here. However the fix is not good, this will need to be adressed separately. It is also better practice to not mangle such fixes into a feature PR. |
Summary
ffDesign_Testswith FreeCAD's native unit-test registry1.0.*and1.1.*M4x0.7Closes #46.
Why this approach
Issue #46 asks for two things: stable coverage of the commands' edge cases and validation across FreeCAD versions. The implementation follows FreeCAD's own test infrastructure instead of building a separate UI automation framework:
FreeCAD -t, and FreeCAD modules such as Mesh register their suites throughFreeCAD.__unit_test__. This PR uses the same registration mechanism inInitGui.pyand runsFreeCAD -t ffDesign_Tests.FreeCADGui, so a display is required even though the tests call the generator functions directly. FreeCAD's own Linux CI workflow runs GUI tests withxvfb-run ... FreeCAD -t; this workflow mirrors that pattern.setup-micromambadocumentation supports creating an environment fromcreate-argsand executing steps withmicromamba-shell. This makes the1.0.*/1.1.*matrix concise and reproducible.Coverage
The shared fixtures assert that generated PartDesign features are non-null, geometrically valid, and have positive volume. Where appropriate, they also assert that material was removed from the source solid.
Compatibility bug found while adding the tests
Running the multi-hole/global-template case on FreeCAD 1.1.1 exposed an existing lookup problem. New thread names include punctuation (for example
M4x0.7), but FreeCAD sanitizes punctuation in an object's internalName.find_rib_template()therefore could not retrieve a template it had just created. The lookup now checks the sanitized internal name and preserves the original thread name as the label fallback. The regression test also verifies that the template is reused rather than duplicated.Validation
Ran 11 tests—OKNo finite test suite can guarantee that every future change is safe, but this establishes a geometry-level regression contract for every current feature. New generators or new behavioral branches can extend the same fixture-based suite.