Skip to content
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 comparisons between bool and int variables #356

Merged
merged 4 commits into from
Jun 22, 2023
Merged

Conversation

tias
Copy link
Collaborator

@tias tias commented Jun 21, 2023

from doc_rework, the test is literally from the new docs (was failing for ortools, includes fix)

I had a look at adding it to test_constraints, but the generator to which to add these cases is not clear to me...

if there is a way to simply test this with all solvers, without having to add it to test_constraints, that would also be fine for me. Even these as is just for ortools is already something.

from doc_rework, the test is literally from the new docs
(was failing for ortools, includes fix)

I had a look at adding it to test_constraints, but the generator to
which to add these cases is not clear to me...

if there is a way to simply test this with all solvers, without having
to add it to test_constraints, that would also be fine for me. Even
these as is just for ortools is already something.
tias added a commit that referenced this pull request Jun 21, 2023
@tias tias added the next release Needed for next release label Jun 21, 2023
@IgnaceBleukx
Copy link
Collaborator

Important one! I'll add it to the test_constraints generators, it should not be too much work (unless many solver fail of course ;) )

@IgnaceBleukx
Copy link
Collaborator

z3 was complaining quite a bit, updated the __add__ code and test_constraints.py to include the new cases.

I had some issues testing Minizinc with the new comparisons, but probably due to my installation... Would be good if someone can rerun the tests for minizinc before merging

@Wout4
Copy link
Collaborator

Wout4 commented Jun 22, 2023

test_constraints.py runs fine for me

@Wout4
Copy link
Collaborator

Wout4 commented Jun 22, 2023

that's the only test file you needed double checked right?

@Wout4
Copy link
Collaborator

Wout4 commented Jun 22, 2023

i do have gecode 6.3 as the default solver though.. Don't think i can easily downgrade to test

@IgnaceBleukx
Copy link
Collaborator

IgnaceBleukx commented Jun 22, 2023

Ok, yes thats the one.
Then this should be ready for review and merge.

@Wout4
Copy link
Collaborator

Wout4 commented Jun 22, 2023

looks good, maybe some obsolete tests now that it's added to test_constraints but more tests doesn't hurt

@Wout4 Wout4 merged commit e5a9c6c into master Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release Needed for next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants