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

Change typecheck test for gcc #347

Merged
merged 2 commits into from
Jun 13, 2023
Merged

Conversation

Dimosts
Copy link
Collaborator

@Dimosts Dimosts commented Jun 12, 2023

For some reason we had 2 different test functions for gcc, the test_global_cardinality_count and the test_gcc. They were doing the same thing. I had only noticed and changed the test_global_cardinality_count in my PR that was merged, and the test_gcc redundant test was left same as in the previous version, and thus now fails with the new syntax.

In this PR I just remove the redundant test test_gcc, as we already have tests in test_global_cardinality_count

@Dimosts Dimosts requested a review from Wout4 June 12, 2023 23:12
@Dimosts Dimosts added the Simple to review Simple change to review, e.g., a oneliner. label Jun 12, 2023
@Wout4
Copy link
Collaborator

Wout4 commented Jun 13, 2023

these tests are a bit different, since they test the typecheck, so different mixes of ints and bools are given as arguments to see if a type-error is thrown. The other tests don't do that

@Dimosts Dimosts changed the title Remove redundant test for gcc (leftover) Change typecheck test for gcc Jun 13, 2023
@Dimosts
Copy link
Collaborator Author

Dimosts commented Jun 13, 2023

ok, changed the typecheck tests then to the new version.

@Dimosts
Copy link
Collaborator Author

Dimosts commented Jun 13, 2023

I now see what happened, the typechecks tests were not introduced when this PR opened, thus there were no typecheck tests to change then, this only occured when merged to main, where in the meanwhile these tests have been introduced.

@IgnaceBleukx
Copy link
Collaborator

Ok, looks good so I'll merge. Once the new master is merged into other open PR's, github will stop complaining about failed tests there as well.

@IgnaceBleukx IgnaceBleukx merged commit d53809a into master Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Simple to review Simple change to review, e.g., a oneliner.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants