Path from ShapeString fails : bug in master c. sept 2019

Here's the place for discussion related to CAM/CNC and the development of the Path module.
vocx
Posts: 2328
Joined: Thu Oct 18, 2018 9:18 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby vocx » Wed Nov 20, 2019 4:17 pm

freman wrote:
Wed Nov 20, 2019 8:22 am
... or is it a case of there being no test included for creating a path from a Bezier ( which to my understanding is the root cause of this failure ) so the Path tests may need one or more extra tests to be included?
Yes, maybe there is a need for a new test that specifically uses a Bezier curve, or a closed Bezier curve. As I've said, I haven't taken a look at the Path tests, but there are a lot. In order to write new tests, you should be familiar with the usage of the workbench, and also familiar with the functions that it calls.
I've seen a list of automatic tasks and tickboxes against PRs but I can't find out how to get to this right now. can you link an example of where that can be seen.
Do you mean in GitHub? See https://github.com/FreeCAD/FreeCAD/pull/2727

Those are created simply by an x inside brackets

Code: Select all

[x] Point one
[x] Point two
But these aren't automatic. The author of the pull request decides to write them or not. They are included by default in every pull request message so that the author remembers to check those points before submitting the pull request. But still, some people never do it, or just plainly remove the checkmarks.
mlampert
Posts: 1435
Joined: Fri Sep 16, 2016 9:28 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby mlampert » Wed Nov 20, 2019 4:27 pm

Path's unit tests are mostly about the underlying functionality and helpers. It's a bit rough to create a robust unit test. We had to disable the basic post-processor unit test because it turned out to be rather finicky. Having said that - that was the first thing I did when I looked at it, searching for an Engrave unit test ;)

Edit: BUT there are quite a few unit tests for orientWire and flipping edges ...
User avatar
freman
Posts: 856
Joined: Tue Nov 27, 2018 10:30 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby freman » Wed Nov 20, 2019 4:48 pm

Thanks for the replies. I obviously don't have the familiarity to start writing tests but at least this may have found a door which needs closing.

It's easy to be wise after the fact and it may not have been obvious to write something generic which would have caught this without seeing it coming, especially with fault condition depending on the characters having internal loops.
Edit: BUT there are quite a few unit tests for orientWire and flipping edges ...
IIRC the output when this engrave path fails had multiple cases of "can't flip edges" or something similar.
Do you mean in GitHub?
Yes, exactly, that's what I was looking for. Thanks.
vocx
Posts: 2328
Joined: Thu Oct 18, 2018 9:18 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby vocx » Wed Nov 20, 2019 6:06 pm

freman wrote:
Wed Nov 20, 2019 4:48 pm
Thanks for the replies. I obviously don't have the familiarity to start writing tests but at least this may have found a door which needs closing.

It's easy to be wise after the fact and it may not have been obvious to write something generic which would have caught this without seeing it coming, especially with fault condition depending on the characters having internal loops.
Yes, and now that you know it fails, you could write a test that is guaranteed to pass. For example, a test for each letter. Or for closed Bezier curves, or something. Then if in the future something changes, and one of these tests fails, then you know something broke that behavior. This is what I mean, in order to write good unit tests, you have to go and test very small details even if they seem insignificant at first. Those tests don't have value until the test fails, and it helps you to troubleshoot the problem.

https://github.com/FreeCAD/FreeCAD/pull/2727

It's not too hard to write the unit tests. See my previous pull request where I write new tests for the Draft Workbench. It's basically a class with methods inside.

Code: Select all

class DraftCreation(unittest.TestCase):
    """Test Draft creation functions."""
...
    def test_line(self):
        """Create a line."""
        operation = "Draft Line"
        _msg("  Test '{}'".format(operation))
        a = Vector(0, 0, 0)
        b = Vector(2, 0, 0)
        _msg("  a={0}, b={1}".format(a, b))
        obj = Draft.makeLine(a, b)
        self.assertTrue(obj, "'{}' failed".format(operation))
That's it. It just creates a line. If the new object exists (it's different from None), then the test will pass. Otherwise the test will fail and report an error.

You need to know what the Draft functions are supposed to do. In this case, Draft.makeLine is supposed to accept two vectors and return a new object. If the input is correct, the output also should be correct. If the internal code of makeLine changes, and the test fails, then we know there is something wrong with our changes.

Currently all unit tests for Draft are in Mod/Draft/TestDraft.py. Once my pull request is merged, they will be in individual modules in Mod/Draft/drafttests/

I don't know Path, but something equivalent could be done. For example, two tests that trace the letters C (open) and D (internal loops).

Code: Select all

class PathCreate(unittest.TestCase):
    def test_C(self):
        obj = Path.makePath("letter_C")
        self.assertTrue(obj, "'failed")
    def test_D(self):
        obj = Path.makePath("letter_D")
        self.assertTrue(obj, "'failed")
Also, I realize there is a similar discussion about testing here, Standardized test shapes. Was this implemented at all?
User avatar
freman
Posts: 856
Joined: Tue Nov 27, 2018 10:30 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby freman » Wed Nov 20, 2019 7:26 pm

Also, I realize there is a similar discussion about testing here, Standardized test shapes. Was this implemented at all?
I did contribut a test shape model there but it was more of a physical test shape for a machine, though it could be used to test s/w too. In fact it did uncover a bug where some geometries lined up exactly.

I don't recall anything happening about making a FC test case out of them.
sliptonic wrote: ping
RatonLaveur
Posts: 382
Joined: Wed Mar 27, 2019 10:45 am

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby RatonLaveur » Thu Nov 21, 2019 6:43 am

Regarding the test shapes, freman I'm pretty sure you were the first and most demonstrative user when you troubleshooted your wood plate project followed by your 3D Surface half-cylinder in metal. I recall you preparing a flange as well.
User avatar
sliptonic
Posts: 1596
Joined: Tue Oct 25, 2011 10:46 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby sliptonic » Fri Nov 22, 2019 3:25 pm

freman wrote:
Wed Nov 20, 2019 7:26 pm
Also, I realize there is a similar discussion about testing here, Standardized test shapes. Was this implemented at all?
I did contribut a test shape model there but it was more of a physical test shape for a machine, though it could be used to test s/w too. In fact it did uncover a bug where some geometries lined up exactly.

I don't recall anything happening about making a FC test case out of them.
sliptonic wrote: ping
I really never intended to include the test shapes in the FC automated test cases. To do so would mean including them in the FreeCAD source tree/repo which probably isn't a good idea. The test shapes were intended more for development, manual testing, and comparison to other CAM applications.