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

Add new expected results files for the Simba ODBC driver. #390

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rhou1
Copy link
Contributor

@rhou1 rhou1 commented Jan 3, 2018

These are empty results files for tests that currently fail with the Simba ODBC driver.
They end in ".fail" so they are ignored by the test framework. When these tests are fixed,
the ".fail" can be removed.

These are empty results files for tests that currently fail with the Simba ODBC driver.
They end in ".fail" so they are ignored by the test framework.  When these tests are fixed,
the ".fail" can be removed.
@rhou1
Copy link
Contributor Author

rhou1 commented Jan 3, 2018

Chun, please review these tests.

@cchang738
Copy link
Contributor

I don't feel it serves a good purpose checking in 800+ empty files just to skip tests, especially these tests are expected to be fixed in a month or two, and these files will be removed. If we only have a few failed tests, then this approach would be acceptable. I almost feel we might as well just leave these tests there and report them as failure if we want to check them in now. Or we can wait a bit until we get most tests passing before check them in. Another approach is to just have one failed_test_file listing all the currently failed tests to skip. This will need some test framework change but should be similar to code added to handle the failed expected files. I will let others to comment. It's not critical to me so I am fine either way.

@rhou1
Copy link
Contributor Author

rhou1 commented Jan 5, 2018

I agree that 800+ empty tests is a lot. But it is temporary.

I am not sure it is useful to implement a new way to handle failed tests if it is only needed for a few months.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants