search

DASSL / ClassDB

An open-source system to let students experiment with relational data
https://dassl.github.io/ClassDB/
Other
8 stars 2 forks source link

Fix testUserMgmt.sql #238

Closed smurthys closed 6 years ago

smurthys commented 6 years ago

The commits in this PR completely revise the test script testUserMgmt.sql. Fixes #224

The revised script is tested against the dev branch. The script needs revision once PR #235 is merged. To be specific, two INSERT queries need to change to match the new schema of table ConnectionActivity.

In the meanwhile, I welcome comments on the changes.

KevinKelly25 commented 6 years ago

It looks good so far and all tests pass on 9.6 and 10.4. The only small thing I see wrong is the indentation on Ln 40.

wildtayne commented 6 years ago

I think this looks good so far. Do we want to merge this now, or wait until #235 is merged and then update this branch again before merging? If we just want to merge, I would be OK with approving this now.

smurthys commented 6 years ago

I feel more comfortable merging after #235 is merged.

EDIT: The test script in this PR also depends on the changes made to resolve #237. So, we should merge after that issue is addressed (if that issue is addressed in M3).

afig commented 6 years ago

Changes made to the tests look good. I agree that it would be a good idea to wait for PRs 235 and 237 to be merged before making the final changes related to those two PRs and merging this in.

smurthys commented 6 years ago

I just pushed some changes to the test script to account for v2.1 schema of tables DDLActivity and ConnectionActivity.

This PR is now ready for review.