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.

Sign up for GitHub

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

Jump to bottom

fix: third-party test #1124

Merged
merged 12 commits into from
Sep 15, 2023
Merged

fix: third-party test #1124

merged 12 commits into from
Sep 15, 2023

Conversation

jiashenC
Copy link
Member

@jiashenC jiashenC commented Sep 14, 2023 •
edited by gaurav274
Loading

  1. Added sqlite testcases to PR testing.
  2. Fixed mysql case sensitivity issue.
  3. Fixed the sqlalchemy uri for each database.

@jiashenC
Copy link
Member Author

Temporarily enable third party tests to ensure CI passes. But, I think we should maybe enable one Postgres even for open PR to prevent future database integration issues being merged to staging?

@jiashenC jiashenC self-assigned this Sep 14, 2023
@jiashenC jiashenC added the Bug 🐞 EVA is not working as expected label Sep 14, 2023
@jiashenC jiashenC added this to the v0.3.5 milestone Sep 14, 2023
@jiashenC
Copy link
Member Author

#1097

#1117

xzdandy
xzdandy previously approved these changes Sep 14, 2023
View reviewed changes
gaurav274
gaurav274 reviewed Sep 14, 2023
View reviewed changes
test/third_party_tests/test_native_executor.py Outdated
@@ -38,7 +38,7 @@ def _create_table_in_native_database(self):
"""USE test_data_source {
CREATE TABLE test_table (
name VARCHAR(10),
Age INT,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intentionally made this change so that we could test the corner case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I missed that. Thanks for point it out.

@gaurav274
Copy link
Member

Temporarily enable third party tests to ensure CI passes. But, I think we should maybe enable one Postgres even for open PR to prevent future database integration issues being merged to staging?

How about sqlite? It does not need any setup overhead and will likely cover most cases.

jiashenC
jiashenC commented Sep 15, 2023
View reviewed changes
script/test/test.sh Outdated
print_error_code $code "FULL TEST"
}

no_coverage_full_test() {
PYTHONPATH=./ python -m pytest -p no:cov test/ -m "not benchmark" --ignore=test/third_party_tests/ --ignore=test/app_tests/
PYTHONPATH=./ python -m pytest -p no:cov test/ test/third_party_tests/test_native_executor.py::NativeExecutorTest::test_should_run_query_in_sqlite-m "not benchmark" --ignore=test/third_party_tests/ --ignore=test/app_tests/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we missing a space here? ... _in_sqlite-m

@jiashenC
Copy link
Member Author

Also is MySQL CI running fine now?

@gaurav274 gaurav274 merged commit 29efc16 into staging Sep 15, 2023
6 checks passed
@gaurav274 gaurav274 deleted the fix-third-party-test branch September 15, 2023 19:14
This was linked to issues Sep 15, 2023
Mysql intergration test failed on staging branch. #1097
Closed
Can't instantiate abstract class NativeStorageEngine with abstract method create #1117
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gaurav274 gaurav274 gaurav274 approved these changes

@xzdandy xzdandy Awaiting requested review from xzdandy

Assignees

@jiashenC jiashenC

Labels
Bug 🐞 EVA is not working as expected
Projects
EVA Public Roadmap ⚡🚀
Archived in project
Milestone
v0.3.5
3 participants
@jiashenC @gaurav274 @xzdandy