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: add and use test settings #938
test: add and use test settings #938
Conversation
@@ -270,12 +270,6 @@ BOOST_AUTO_TEST_CASE( test_difference_linestring_linestring ) | |||
from_wkt<ML>("MULTILINESTRING((0 0,0.5 0))"), | |||
"lldf16"); | |||
|
|||
tester::apply |
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.
I understand that all the reverse tests -r
are removed as duplicates and the rest are disactivated for now and fixed in the future. Are the -r
tests failing now too? Are they going to be fixed in the future as well?
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.
- the
-r
cases are indeed removed as duplicate - some of the
-r
cases are now failing, and only they are disabled - yes, this was the first step to see what's going wrong there and I will continue next Wednesday
- so this PR is a first step - it was the last step in letting all tests pass, without rescaling, in set_operations and buffer
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.
I don't understand what is the rationale behind removing them? These are not duplicates since the inputs are different. The fact that some of them fails now is a good reason why they should stay. It's because they might fail in the future after changing something else. This is what tests are for, not to pass but to fail if something is wrong.
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.
what I was understood is that the tester function will be responsible to test the "reverse" case thus in the scenario the -r
cases are duplicates. But I strongly agree that the reverse cases should be always tested.
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.
@vissarion Ok thanks. So they are not removed but instead various settings
like test_normal_reverse
are used inside.
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.
Sure, conform description
The reverse cases where already tested inside. So they were always duplicate. But now they are configurable.
#if ! defined(BOOST_GEOMETRY_USE_RESCALING) && ! defined(BOOST_GEOMETRY_TEST_FAILURES) | ||
// Cases failing without rescaling when first linestring is reversed | ||
settings.test_reverse_normal = false; | ||
settings.test_reverse_reverse = false; |
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.
@vissarion for example you can see here that only two of the cases are excluded from the test, and only for if rescaling is not used. So in the default case, everything is still tested as it was before (now without duplicates)
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.
Thanks for the explanations.
This
There are still some cases failing without rescaling, to be investigated further