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

Integration test CI #37

Merged
merged 31 commits into from
Aug 5, 2024
Merged

Integration test CI #37

merged 31 commits into from
Aug 5, 2024

Conversation

wangpatrick57
Copy link
Member

@wangpatrick57 wangpatrick57 commented Jul 24, 2024 •
edited
Loading

Summary: Added a CI for running integration tests. Fixed a few bugs to get it working.

Demo:
Successful CI
Screenshot 2024-08-04 at 21 08 29

Details:

  • Made the runner use the runner user instead of root so we can start Postgres in it.
  • Made new DBGym config for integration test so that the ports and workspace don't conflict (yay config file!). This is not currently necessary since we run it in a Docker container but it's a good precaution.
  • In the integration test, we run the full chain (with replay) twice so that we test it both without and with cached results in the workspace. We make sure to delete the workspace before running twice (again not necessary because we're using Docker but is a good precaution).

@wangpatrick57
added integration test to ci
5e9e19e
@wangpatrick57
fixed bug of result_path var not existing
d7b83d8
@wangpatrick57
fixed type is Path checks to use isintance
a472bd3
@wangpatrick57
fixed bug where var wasn't path
d252e41
@wangpatrick57
replaced most uses of os.path.samefile with Path.samefile
62516b4
@wangpatrick57
fixed bug where link_result didn't add .link to the end of names
6889d1e
@wangpatrick57
fixed bug where save_file() was still taking in regular paths
42b5727
@wangpatrick57
changed save_file() to what it used to be
7d1cab4
@wangpatrick57
using parent_dpath_of_path and basename_of_path instead of os.path fu…
837fadc 
…nctions
@wangpatrick57
fixed comment
05a9497
@wangpatrick57
fixed bug of putting dbdata in dbgym_tmp_path during embedding datagen
7dd2bd5
@wangpatrick57
fixed bug where we were initializing ray with the boot redis port
5b98b6e
@wangpatrick57
moved stuff to integration_test.sh
979f8ab
@wangpatrick57
added integtest with its own config
ca7dc7d
@wangpatrick57
made integration_test.sh export the config path envvar
9dddf56
@wangpatrick57 wangpatrick57 marked this pull request as ready for review July 24, 2024 00:02
@wangpatrick57 wangpatrick57 changed the title Integtest ci Integration test CI Aug 5, 2024
@wangpatrick57 wangpatrick57 requested a review from lmwnshn August 5, 2024 04:13
lmwnshn
lmwnshn approved these changes Aug 5, 2024
View reviewed changes
Copy link
Collaborator

@lmwnshn lmwnshn left a comment

Choose a reason for hiding this comment

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

Some small leftovers. Nice work!

creations.txt Outdated Show resolved Hide resolved
hi Outdated Show resolved Hide resolved
@wangpatrick57 wangpatrick57 merged commit 9ef1ee6 into cmu-db:main Aug 5, 2024
1 check passed
@wangpatrick57 wangpatrick57 deleted the integtest-ci branch October 18, 2024 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmwnshn lmwnshn lmwnshn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wangpatrick57 @lmwnshn