Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 510906

Summary:	Preferences test failures
Product:	[ECD] Orion	Reporter:	Remy Suen <remy.suen>
Component:	Node	Assignee:	Remy Suen <remy.suen>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	steve_northover
Version:	13.0
Target Milestone:	15.0
Hardware:	PC
OS:	Windows 10
Whiteboard:

Description Remy Suen

2017-01-23 15:58:17 EST

Note that you won't see these if you just try to run the whole test suite due to the fallout caused by the failing bug 500579.

Repeatedly running this test may cause additional failures (presumably due to cleanup or file locking issues?).

-------------------

> npm test

> Orion@0.1.0 test orion.client\modules\orionode
> mocha --reporter spec



  prefs
    when NO prefs.json exists
      and we GET a nonexistent single key
        √ should receive 404 (136ms)
      and we GET a nonexistent node
        √ should receive empty node
    when prefs.json exists
      and we GET a nonexistent single key
        √ should receive 404
      and we GET a nonexistent node
        √ should receive empty node
      and we GET a single key
        √ should receive just that property
      and we GET a node
        √ should receive the entire node
      and we PUT a single key
        1) should update the value
      and we PUT an entire node
        2) should update the value
      and we DELETE a single key
        √ should not have the deleted key
      and we DELETE entire node
        √ should be empty
      and we DELETE a non-existent node
        √ should have no effect


  9 passing (5s)
  2 failing

  1) prefs when prefs.json exists and we PUT a single key should update the value:

      AssertionError: expected { bar: 123, qux: 'q' } to deeply equal { bar: 'modified', qux: 'q' }
      + expected - actual

       {
      -  "bar": 123
      +  "bar": "modified"
         "qux": "q"
       }

      at Assertion.assertEqual (orion.client\modules\orionode\node_modules\chai\lib\chai\core\assertions.js:485:19)
      at Assertion.ctx.(anonymous function) [as equal] (orion.client\modules\orionode\node_modules\chai\lib\chai\utils\addMethod.js:41:25)
      at orion.client\modules\orionode\test\test-prefs.js:134:31
      at tryCatcher (orion.client\modules\orionode\node_modules\bluebird\js\release\util.js:16:23)
      at Promise._settlePromiseFromHandler (orion.client\modules\orionode\node_modules\bluebird\js\release\promise.js:510:31)
      at Promise._settlePromise (orion.client\modules\orionode\node_modules\bluebird\js\release\promise.js:567:18)
      at Promise._settlePromise0 (orion.client\modules\orionode\node_modules\bluebird\js\release\promise.js:612:10)
      at Promise._settlePromises (orion.client\modules\orionode\node_modules\bluebird\js\release\promise.js:691:18)
      at Async._drainQueue (orion.client\modules\orionode\node_modules\bluebird\js\release\async.js:133:16)
      at Async._drainQueues (orion.client\modules\orionode\node_modules\bluebird\js\release\async.js:143:10)
      at Immediate.Async.drainQueues [as _onImmediate] (orion.client\modules\orionode\node_modules\bluebird\js\release\async.js:17:14)

  2) prefs when prefs.json exists and we PUT an entire node should update the value:

      AssertionError: expected { bar: 123, qux: 'q' } to deeply equal { howdy: 'partner' }
      + expected - actual

       {
      -  "bar": 123
      -  "qux": "q"
      +  "howdy": "partner"
       }

      at Assertion.assertEqual (orion.client\modules\orionode\node_modules\chai\lib\chai\core\assertions.js:485:19)
      at Assertion.ctx.(anonymous function) [as equal] (orion.client\modules\orionode\node_modules\chai\lib\chai\utils\addMethod.js:41:25)
      at orion.client\modules\orionode\test\test-prefs.js:150:31
      at tryCatcher (orion.client\modules\orionode\node_modules\bluebird\js\release\util.js:16:23)
      at Promise._settlePromiseFromHandler (orion.client\modules\orionode\node_modules\bluebird\js\release\promise.js:510:31)
      at Promise._settlePromise (orion.client\modules\orionode\node_modules\bluebird\js\release\promise.js:567:18)
      at Promise._settlePromise0 (orion.client\modules\orionode\node_modules\bluebird\js\release\promise.js:612:10)
      at Promise._settlePromises (orion.client\modules\orionode\node_modules\bluebird\js\release\promise.js:691:18)
      at Async._drainQueue (orion.client\modules\orionode\node_modules\bluebird\js\release\async.js:133:16)
      at Async._drainQueues (orion.client\modules\orionode\node_modules\bluebird\js\release\async.js:143:10)
      at Immediate.Async.drainQueues [as _onImmediate] (orion.client\modules\orionode\node_modules\bluebird\js\release\async.js:17:14)

Comment 1 Remy Suen

2017-03-10 03:33:05 EST

(In reply to Remy Suen from comment #0)
>   1) prefs when prefs.json exists and we PUT a single key should update the
> value:
> 
>       AssertionError: expected { bar: 123, qux: 'q' } to deeply equal { bar:
> 'modified', qux: 'q' }
>       + expected - actual
> 
>        {
>       -  "bar": 123
>       +  "bar": "modified"
>          "qux": "q"
>        }

This first failure is due to the asynchronous nature of the preferences implementation. If I change this write call to be synchronous then the test will pass without any problems.

https://github.com/eclipse/orion.client/blob/master/modules/orionode/lib/controllers/prefs.js#L174

Comment 2 Remy Suen

2017-03-10 04:02:05 EST

(In reply to Remy Suen from comment #1)
> https://github.com/eclipse/orion.client/blob/master/modules/orionode/lib/
> controllers/prefs.js#L174

This link will get old with time as it's pointing to master. Here's the "current" relevant commit as of the time of submission of this comment.

https://github.com/eclipse/orion.client/blob/3a09df53a3c5bd7c06786588337de8f74009b332/modules/orionode/lib/controllers/prefs.js#L174

Comment 3 Steve Northover

2017-03-30 16:58:40 EDT

"synchronous" is usually bad but this is just in test code, no?

Comment 4 Remy Suen

2017-03-30 17:12:21 EDT

(In reply to Steve Northover from comment #3)
> "synchronous" is usually bad but this is just in test code, no?

No. Sorry I didn't make that clear in comment 1. I meant if I changed the write code to be synchronous in the actual preferences implementation.

Comment 5 Steve Northover

2017-03-30 17:13:41 EDT

Ok, I am thinking we are never doing this.

Comment 6 Remy Suen

2017-04-05 07:27:40 EDT

I "fixed" this by adding a delay to the test between setting up the preferences file and running the assertions. The delay is a short 10 milliseconds but it seems to be doing the trick...
https://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=4ab560f10f650264b0bc067c81898991e91509cd