-
Notifications
You must be signed in to change notification settings - Fork 109
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
Test OcdFileImport::fillPathCoords #2270
Conversation
Move expected data closer to input data. Format data more readable. Avoid redundant passing of element counts. Test path coord processing both for areas and lines.
|
||
void FileFormatTest::ocdPathImportTest_data() | ||
{ | ||
#define C(x) ((int)((unsigned int)(x)<<8)) // FIXME: Not the same as in export |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as we don't check the actual x/y values, it doesn't need action.
void FileFormatTest::ocdPathImportTest_data() | ||
{ | ||
#define C(x) ((int)((unsigned int)(x)<<8)) // FIXME: Not the same as in export | ||
constexpr auto ocd_flag_gap = 8; // TODO: implement as Ocd::OcdPoint32::FlagGap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
TestOcdFileImport::OcdImportedPathObject path_object; | ||
ocd_v12_import.fillPathCoords(&path_object, personality == Area, points.size, points.data); | ||
QVERIFY(path_object.getRawCoordinateVector().size() > 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a time when OcdPointsView
and FlagsView
returned empty views, turning the test into a no-op. I prefer to leave it in.
Ping @dl3sdo. This has a more direct impact on what can be merged next. |
@dg0yt: Just to understand it right: this unit test is intentionally designed for the 'old' behaviour, i.e. merging #2224 would require an update of this unit test.
passes and should not pass after #2224. |
Kai: this unit test is excellent: it's understandable, easily extensible and it documents the mapping of .ocd coords and attributes to Mapper's coords and attributes. Thank you for the effort you spent on it! I added another test case but cannot push to your branch, please pick it from ab12494 |
Yes, for the moment. It is sort-of a documentation of what is implemented now. |
The test case contains an Ocd::OcdPoint32::FlagDash property which was yet untested as well as a combination of bezier and straight line segments. The area itself contains a hole which itself contains a hole.
Extended version of the proposed test, #2224 (comment)