Page MenuHomePhabricator
Create Task
Maniphest T99696

cleanup and refactor test/index.js
Closed, ResolvedPublic
Actions

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.

Related Objects
Search...

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 added a subscriber: Hardikj.May 19 2015, 6:54 PM
Hardikj claimed this task.May 19 2015, 6:58 PM
Hardikj set Security to None.
Hardikj added a comment.May 19 2015, 11:05 PM

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 
        -  ..
• mobrovac added a subscriber: • mobrovac.May 20 2015, 7:46 AM

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
In T99696#1298514, @mobrovac wrote:

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 added a parent task: T103406: move restbase-mod-table-cassandra tests and spec into a separate module.Jun 24 2015, 3:39 PM
Eevans added a comment.Jul 20 2015, 4:48 PM

This is complete in https://github.com/wikimedia/restbase-mod-table-cassandra/pull/114

Eevans closed this task as Resolved.Jul 20 2015, 4:48 PM
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL