Cracks

[aeweidne]

this would be the minimum viable cracks integration, the build should fail when cracks detects that the tests associated with our last release are failing against the current code base.

Failing tests may be (on occasion) a false positive, but it is better to be safe than sorry. Tests should be testing the API and not the implementation anyway, which would be the biggest cause of false positives using this tool.

In general, it is easy to think about API breaking changes in relation to tests:

1. Existing test cases really shouldn't need to be changed for a bug fix -- that is a red flag that the test is testing implementation details and not the API, or that the test was broken to begin with. This would be a patch
2. If you add new test cases (for a new feature, not as a result of a bugfix that wasn't captured by a previous test case), then that is a minor.
3. If you add new functionality and that results in test rewrites, that is a major release, unless the test cases are testing implementation details and not the API.

It looks like we may have caught a breaking change around the rrManager stuff as this build is failing due to cracks.

[sam1463]

The cracks failure should be at least in part due to the fact that the tests are relying on setting/changing variables manually, rather than just calling the APIs. It's quite possible that there were breaking changes in addition to this. However, I assume tests should not be directly setting variables (e.g. watson_nlc.opts.classifierName = trainingClassifier;), which seems to test implementation. At least some of the failures in the checks above are due to the fact that the implementation has changed (using serviceManager via nlcManager, for example), so setting the variable manually no longer has the desired effect. Is this a reasonable thing for tests to be doing? I think it would make more sense for tests to reinitialize an instance of NLCManager, for example, in order to change options, that way the test is only calling the APIs and the Initialization method, and not testing implementation (that being said, the new tests still are manually changing variables, so they'll need to be changed if this is the direction we're going in).

[aeweidne]

@sam1463 I agree so long as the variables are never changed by hand by consumers of the library. If they are, then it's definitely something that we want to test. If they don't, then the tests should be changed to reflect the API usage instead of modifying private fields directly (calling the functions that the user would call to change those fields). I would have to sit down more with the cognitive-lib to understand its API to make that call myself, but would be happy to do so if that is needed.

[sam1463]

Just spoke with Jorge, and he agrees that the initializer is what should actually be called, and that libraries using cog-lib are only setting variables using the initializer, not changing them on the fly. So I think in this specific case, we should be making the tests rely only on the initializer and the APIs, since that's all that users are using, so changing variables manually is testing implementation and not a real use case. To go about changing issues like this, should we first make a PR to change the tests to not rely on implementation, that way cracks won't fail since the functionality doesn't fail, then make a new PR to change the functionality, and the new PR would also not fail since cracks would check against the new tests, not the old ones?

[aeweidne]

In this specific situation since we haven't fully ironed out how we are going to work cracks in and we have already delivered the functionality, I would say make another issue + PR for fixing the tests to use the APIs. I will work on an integration scheme for cracks so that we do not have to make separate PRs/Issues for tests and functionality. We definitely want to strive towards always checking in tests with functionality.

Sorry if my above message was confusing.

[coveralls]

Changes Unknown when pulling 4c44cad on cracks into * on master*.

[ibmcloudsolutions]

A potential API Breaking change was found in this PR:

> [email protected] test /home/travis/build/ibm-cloud-solutions/hubot-ibmcloud-cognitive-lib
> . test/.env && mocha test



  Testing of database.
    Test database info

      ✓ should respond with database info
    Test adding to an existing document in the database

      ✓ should update the existing document in the database with a `dummy` field
    Test posting documents in the database

      ✓ should create a  document in the database with a `classification` field

      ✓ should create a  document in the database with a `logs` field

      ✓ should create a document in the database with thresholds

  Testing NLC Configuration
    Verify class-related data is stored properly

      ✓ Verify getAllClasses() (52ms)

      ✓ Verify getAllClasses(approvedAfterDate) - with new Date() object

      ✓ Verify getAllClasses(approvedAfterDate) - with date in ms

      ✓ Verify getClassEmitTarget

      ✓ Verify getClassEmitTarget for invalid class

      ✓ Verify getClassEmitTarget contains class description
    Verify Auto Approve setter/getter

      ✓ Verify getAutoApprove contains correct value

      ✓ Verify setAutoApprove sets correct value

      ✓ Verify setAutoApprove sets correct value
    Verify entity function setter/getter

      ✓ Verify entity function setter/getter correct value

  Test the NLCManager library

    ✓ should classify statement as weather (62ms)

    ✓ Should monitor a classifier while it is being trained and delete old classifiers when training completes

    ✓ should successfully get the status of most recent available classifier

    1) should successfully get status of most recent training classifier

    ✓ should successfully list filtered classifiers in correct order

    ✓ should successfully get the current classifier

    ✓ Should not train existing classifier

    2) Should start training classifier with provided training_data

    3) Should start training classifier with dynamic training_data

    ✓ should successfully get training data for classifier
    Negative tests

      ✓ should fail getting the status of classifier

      4) should fail to get status of classifier

      5) should fail to get an available/training classifier

      ✓ should fail to get training data for a classifier that doesn't exist
    NLC 500 errors

      ✓ should fail to list all classifiers

      ✓ should fail to train a classifier

  Test the RRManager library
    Test the solr cluster methods

      6) "before all" hook
    Test the ranker methods

      7) "before all" hook
    Negative tests

      8) "before each" hook for "should fail getting the status of ranker"

  Testing initial load of database
    Test loading master NLC JSON file

      ✓ should test database has been loaded with seed data

      ✓ should test database class responses with seed data from a JSON file format (53ms)

      ✓ should test database parameter responses with seed data from a JSON file format


  29 passing (4s)
  8 failing

  1) Test the NLCManager library should successfully get status of most recent training classifier:
     TypeError: Cannot read property '_options' of undefined
      at Context.<anonymous> (test/nlcManager.test.js:75:17)

  2) Test the NLCManager library Should start training classifier with provided training_data:
     Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.


  3) Test the NLCManager library Should start training classifier with dynamic training_data:
     Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.


  4) Test the NLCManager library Negative tests should fail to get status of classifier:
     TypeError: Cannot read property '_options' of undefined
      at Context.<anonymous> (test/nlcManager.test.js:110:18)

  5) Test the NLCManager library Negative tests should fail to get an available/training classifier:
     TypeError: Cannot read property '_options' of undefined
      at Context.<anonymous> (test/nlcManager.test.js:119:18)

  6) Test the RRManager library Test the solr cluster methods "before all" hook:
     TypeError: Parameter 'url' must be a string, not undefined
      at Url.parse (url.js:90:11)
      at Object.urlParse [as parse] (url.js:84:5)
      at new Scope (node_modules/nock/lib/scope.js:52:25)
      at startScope (node_modules/nock/lib/scope.js:27:10)
      at Object.module.exports.setupSolrMockery (test/rrManager.mock.js:137:21)
      at Context.<anonymous> (test/rrManager.test.js:56:18)

  7) Test the RRManager library Test the ranker methods "before all" hook:
     TypeError: Parameter 'url' must be a string, not undefined
      at Url.parse (url.js:90:11)
      at Object.urlParse [as parse] (url.js:84:5)
      at new Scope (node_modules/nock/lib/scope.js:52:25)
      at startScope (node_modules/nock/lib/scope.js:27:10)
      at Object.module.exports.setupMockery (test/rrManager.mock.js:30:17)
      at Context.<anonymous> (test/rrManager.test.js:168:18)

  8) Test the RRManager library "before each" hook for "should fail getting the status of ranker":
     Error: Argument error: api_key or username and password were not specified
      at RetrieveAndRankV1.BaseService.initCredentials (node_modules/watson-developer-cloud/lib/base_service.js:87:17)
      at RetrieveAndRankV1.BaseService (node_modules/watson-developer-cloud/lib/base_service.js:45:18)
      at new RetrieveAndRankV1 (node_modules/watson-developer-cloud/retrieve-and-rank/v1.js:36:15)
      at Object.defineProperty.value (node_modules/watson-developer-cloud/index.js:87:16)
      at new RRManager (src/lib/rrManager.js:45:19)
      at init (test/rrManager.test.js:40:10)
      at Context.<anonymous> (test/rrManager.test.js:51:15)




npm ERR! Test failed.  See above for more details.

[Error: Old tests failed. Breaking Change detected.]

[ibmcloudsolutions]

A potential API Breaking change was found in this PR:

> [email protected] test /home/travis/build/ibm-cloud-solutions/hubot-ibmcloud-cognitive-lib
> . test/.env && mocha test



  Testing of database.
    Test database info

      ✓ should respond with database info
    Test adding to an existing document in the database

      ✓ should update the existing document in the database with a `dummy` field
    Test posting documents in the database

      ✓ should create a  document in the database with a `classification` field

      ✓ should create a  document in the database with a `logs` field

      ✓ should create a document in the database with thresholds

  Testing NLC Configuration
    Verify class-related data is stored properly

      ✓ Verify getAllClasses()

      ✓ Verify getAllClasses(approvedAfterDate) - with new Date() object

      ✓ Verify getAllClasses(approvedAfterDate) - with date in ms

      ✓ Verify getClassEmitTarget

      ✓ Verify getClassEmitTarget for invalid class

      ✓ Verify getClassEmitTarget contains class description
    Verify Auto Approve setter/getter

      ✓ Verify getAutoApprove contains correct value

      ✓ Verify setAutoApprove sets correct value

      ✓ Verify setAutoApprove sets correct value
    Verify entity function setter/getter

      ✓ Verify entity function setter/getter correct value

  Test the NLCManager library

    ✓ should classify statement as weather (43ms)

    ✓ Should monitor a classifier while it is being trained and delete old classifiers when training completes

    ✓ should successfully get the status of most recent available classifier

    1) should successfully get status of most recent training classifier

    ✓ should successfully list filtered classifiers in correct order

    ✓ should successfully get the current classifier

    ✓ Should not train existing classifier

    2) Should start training classifier with provided training_data

    3) Should start training classifier with dynamic training_data

    ✓ should successfully get training data for classifier
    Negative tests

      ✓ should fail getting the status of classifier

      4) should fail to get status of classifier

      5) should fail to get an available/training classifier

      ✓ should fail to get training data for a classifier that doesn't exist
    NLC 500 errors

      ✓ should fail to list all classifiers

      ✓ should fail to train a classifier

  Test the RRManager library
    Test the solr cluster methods

      6) "before all" hook
    Test the ranker methods

      7) "before all" hook
    Negative tests

      8) "before each" hook for "should fail getting the status of ranker"

  Testing initial load of database
    Test loading master NLC JSON file

      ✓ should test database has been loaded with seed data

      ✓ should test database class responses with seed data from a JSON file format (51ms)

      ✓ should test database parameter responses with seed data from a JSON file format


  29 passing (4s)
  8 failing

  1) Test the NLCManager library should successfully get status of most recent training classifier:
     TypeError: Cannot read property '_options' of undefined
      at Context.<anonymous> (test/nlcManager.test.js:75:17)

  2) Test the NLCManager library Should start training classifier with provided training_data:
     Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.


  3) Test the NLCManager library Should start training classifier with dynamic training_data:
     Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.


  4) Test the NLCManager library Negative tests should fail to get status of classifier:
     TypeError: Cannot read property '_options' of undefined
      at Context.<anonymous> (test/nlcManager.test.js:110:18)

  5) Test the NLCManager library Negative tests should fail to get an available/training classifier:
     TypeError: Cannot read property '_options' of undefined
      at Context.<anonymous> (test/nlcManager.test.js:119:18)

  6) Test the RRManager library Test the solr cluster methods "before all" hook:
     TypeError: Parameter 'url' must be a string, not undefined
      at Url.parse (url.js:90:11)
      at Object.urlParse [as parse] (url.js:84:5)
      at new Scope (node_modules/nock/lib/scope.js:52:25)
      at startScope (node_modules/nock/lib/scope.js:27:10)
      at Object.module.exports.setupSolrMockery (test/rrManager.mock.js:137:21)
      at Context.<anonymous> (test/rrManager.test.js:56:18)

  7) Test the RRManager library Test the ranker methods "before all" hook:
     TypeError: Parameter 'url' must be a string, not undefined
      at Url.parse (url.js:90:11)
      at Object.urlParse [as parse] (url.js:84:5)
      at new Scope (node_modules/nock/lib/scope.js:52:25)
      at startScope (node_modules/nock/lib/scope.js:27:10)
      at Object.module.exports.setupMockery (test/rrManager.mock.js:30:17)
      at Context.<anonymous> (test/rrManager.test.js:168:18)

  8) Test the RRManager library "before each" hook for "should fail getting the status of ranker":
     Error: Argument error: api_key or username and password were not specified
      at RetrieveAndRankV1.BaseService.initCredentials (node_modules/watson-developer-cloud/lib/base_service.js:87:17)
      at RetrieveAndRankV1.BaseService (node_modules/watson-developer-cloud/lib/base_service.js:45:18)
      at new RetrieveAndRankV1 (node_modules/watson-developer-cloud/retrieve-and-rank/v1.js:36:15)
      at Object.defineProperty.value (node_modules/watson-developer-cloud/index.js:87:16)
      at new RRManager (src/lib/rrManager.js:45:19)
      at init (test/rrManager.test.js:40:10)
      at Context.<anonymous> (test/rrManager.test.js:51:15)




npm ERR! Test failed.  See above for more details.

[Error: Old tests failed. Breaking Change detected.]

[coveralls]

Changes Unknown when pulling 780c0bb on cracks into * on master*.