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: eval_general should assume by default that pandas does not throw an error #6016

Closed
mvashishtha opened this issue Apr 17, 2023 · 3 comments · Fixed by #6954
Closed

TEST: eval_general should assume by default that pandas does not throw an error #6016

mvashishtha opened this issue Apr 17, 2023 · 3 comments · Fixed by #6954
Assignees
Labels
P1 Important tasks that we should complete soon Testing 📈 Issues related to testing

Comments

@mvashishtha
Copy link
Collaborator

mvashishtha commented Apr 17, 2023

right now eval_general with default parameters will not raise an assertion if both pandas and modin raise an error, even if that error is due to a mistake in writing the test code. This is dangerous behavior and it has silently swallowed mistakes in test code before. We should have to explicitly state that we are looking for an error.

Right now it's possible to work around this by setting raising_exceptions=(Exception,), but that should be the default.

@modin-project/modin-contributors @modin-project/modin-core @anmyachev I would like your thoughts on this.

@mvashishtha mvashishtha added Testing 📈 Issues related to testing P1 Important tasks that we should complete soon labels Apr 17, 2023
@anmyachev
Copy link
Collaborator

@mvashishtha I agree, we should change the default behaviour.

@dchigarev
Copy link
Collaborator

+1 on this change

@vnlitvinov
Copy link
Collaborator

I would add that we may want to check that error messages are "close" (I don't have a good enough definition of "close" here, though).

The case my proposal is aimed at is handling a case when e.g. pandas throws an AttributeError but we've made a typo in test code (and so are also getting AttributeError but not the one we're expecting).

@anmyachev anmyachev self-assigned this Jan 19, 2024
anmyachev added a commit to anmyachev/modin that referenced this issue Feb 21, 2024
…ptions by default

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>
anmyachev added a commit to anmyachev/modin that referenced this issue Feb 22, 2024
…ptions by default

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>
anmyachev added a commit to anmyachev/modin that referenced this issue Feb 27, 2024
…ptions by default

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>
anmyachev added a commit to anmyachev/modin that referenced this issue Mar 1, 2024
…ptions by default

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>
YarShev pushed a commit that referenced this issue Mar 14, 2024
…ault (#6954)

Signed-off-by: Anatoly Myachev <anatoly.myachev@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Important tasks that we should complete soon Testing 📈 Issues related to testing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants