Page MenuHomePhabricator

cleanup and refactor test/index.js
Closed, ResolvedPublic

Description

test/index.js has become an unwieldy catch-all of sorts, that could do with some cleanup and refactoring. Worth considering:

  • The elimination of string literal UUID representations. Ff an attribute was persisted using dbu.testTidFromDate(new Date(datestr))), perform the subsequent assertion the same way, or better yet, reuse a well named variable.
  • Restructure the tests to be less monolithic with groupings that each encapsulate some subset of functionality. For example, static columns, order-by, secondary indexes, etc.

Event Timeline

Eevans created this task.May 19 2015, 6:40 PM
Eevans raised the priority of this task from to Low.
Eevans updated the task description. (Show Details)
Eevans added a project: RESTBase-Cassandra.
Eevans added a subscriber: Eevans.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 19 2015, 6:40 PM
Hardikj claimed this task.May 19 2015, 6:58 PM
Hardikj set Security to None.

Ideas below are inspired from the directory structure of the tests in restbase

This is how the current directory looks like

test/
  -  dump
  -  dbutils.js
  -  index.js  
  -  revision_policies.js
  -  schema_migrations.js
  -  jshint.spec.js
  -  test_router.js
  -  unused_queries.txt

One option could be

test/
  -  dump/
  -  utils/
     -  jshint.spec.js
     -  test_router.js
     -  unused_queries.txt
  -  tests/
     -  dbutils.js 
     -  revision_policies.js
     -  schema_migrations.js
     -  database/
        -  static.js
        -  secondaryIndexes.js
        -  simple.js
        -  varint.js
  -  index.js

Other alternative would be group the test on the basis of queries

/test/tests/datatbase/
        -  get.js
        -  put.js
        -  delete.js 
        -  ..

I favour the first option, with a twist:

test/
 - dump/
 - utils/
   - jshint.spec.js
   - test_router.js
   - unused_queries.txt
 - feature/
   - data/
     - static.js
     - secondaryIndexes.js
     - simple.js
     - varint.js
   - helpers/
     - dbutils.js
   - meta/
     - revision_policies.js
     - schema_migrations.js
Eevans added a comment.EditedMay 20 2015, 4:32 PM

I favour the first option, with a twist:

test/
 - dump/
 - utils/
   - jshint.spec.js
   - test_router.js
   - unused_queries.txt
 - feature/
   - data/
     - static.js
     - secondaryIndexes.js
     - simple.js
     - varint.js
   - helpers/
     - dbutils.js
   - meta/
     - revision_policies.js
     - schema_migrations.js

I'd be good with this, but just to stir the pot a bit, another variation would be:

test/
  - dump/
  - utils/
    - jshint.spec.js
    - test_router.js
    - unused_queries.txt
  - functional/
    - static.js
    - secondaryIndexes.js
    - simple.js
    - varint.js
    - revision_policies.js
    - schema_migrations.js
  - unit/
    - dbutils.js

I don't feel super-passionate about this though, either way would make for a major improvement.

Eevans closed this task as Resolved.Jul 20 2015, 4:48 PM