DASSL / ClassDB

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

File names for test scripts are inconsistent (W) #210

Open afig opened 6 years ago

afig commented 6 years ago

There is an inconsistency on whether test scripts have "add" in their filenames. I would advise that they do not, since the tests are testing the feature's behavior, not necessarily the successful addition of the feature. Additionally, the naming of runAllPrivilegeTests.psql is not consistent with other scripts, something like testPrivileges.psql may be more appropriate. Likewise, the associated README should be renamed. If the privilege test suite is renamed to testPrivileges.psql then the README should be renamed to testPrivilegesREADME.txt.

There are other issues related to the structure and behavior of test files, including #207, but they do not have to be addressed in M3.

Current directory structure of the /tests directory:

├── importStudents
│   ├── README.md
│   └── testBannerRoster.csv
├── privileges
│   ├── 0_setup.sql
│   ├── 1_instructorPass.sql
│   ├── 2_studentPass.sql
│   ├── 3_dbmanagerPass.sql
│   ├── 4_instructorPass2.sql
│   ├── 5_instructorFail.sql
│   ├── 6_studentFail.sql
│   ├── 7_dbmanagerFail.sql
│   ├── 8_cleanup.sql
│   ├── runAllPrivilegeTests.psql
│   └── testPrivilegesREADME.txt
├── testAddConnectionActivityLoggingCleanup.sql
├── testAddConnectionActivityLogging.psql
├── testAddDDLActivityLogging.sql
├── testClassDBRolesMgmt.sql
├── testHelpers.sql
├── testRoleBaseMgmt.sql
└── testUserMgmt.sql
smurthys commented 6 years ago

I agree with the issue description. I feel a lone README file in any directory can just be called README.

I believe the issue of the "add" prefix has been discussed previously and thus some test scripts do not have that prefix.

wildtayne commented 6 years ago

I also agree with @afig's analysis on test script names. Renaming any lone README to README also has the advantage of displaying in the README view below the file list on GitHub.