TranscendComputing / StackStudio

The front end for private and public clouds.
Other
2 stars 0 forks source link

Build dependency on config/backend.json #123

Closed cstewart87 closed 10 years ago

cstewart87 commented 10 years ago

I was required to update .travis.yml to create config/backend.json in order for builds to pass.

See #122 (1b8ca7700e5439b73ad315ddcab6659edcfc3125)

I'm assuming this shouldn't be the case. If it is, just close as the failed builds have been resolved.

lifeBCE commented 10 years ago

Should this be closed?

cstewart87 commented 10 years ago

There's still a dependency here, I believe. Is that OK? It seems like the tests shouldn't depend on the backend file.

pjschmidt3 commented 10 years ago

They probably shouldn't, I guess, but it probably wouldn't hurt either, since this way it's at least sort of testing that the file exists On Jun 6, 2014 9:26 AM, "Curtis Stewart" notifications@github.com wrote:

There's still a dependency here, I believe. Is that OK? It seems like the tests shouldn't depend on the backend file.

— Reply to this email directly or view it on GitHub https://github.com/TranscendComputing/StackStudio/issues/123#issuecomment-45342233 .

lifeBCE commented 10 years ago

Not too familiar with what travis is doing. What gave you the indication this was required?

cstewart87 commented 10 years ago

I thought the links I added to this issue had some failure reference, apparently not. I do recall tests not working on a fresh clone of the repo, and seeing some indication that the backend.json file was the only difference, or some error specifically stating that it was missing.

cstewart87 commented 10 years ago

Removing the backend.json config:

curtstewart@curtis-mbp:StackStudio$ mv config/backend.json config/backend.json.old
curtstewart@curtis-mbp:StackStudio$ npm test

> StackStudio@2.0.0-rc2 test /Users/curtstewart/Git/GitHub/Work/StackStudio
> grunt test

[supressed output]

Running "jasmine:test" (jasmine) task
Testing jasmine specs via phantom
>> TypeError: 'undefined' is not a function (evaluating 'd.widget') at
>> http:/cdn.jsdelivr.net/jquery.multiselect/1.13/jquery.multiselect.min.js:20
>> http:/cdn.jsdelivr.net/jquery.multiselect/1.13/jquery.multiselect.min.js:20
>> TypeError: 'undefined' is not a function (evaluating 'a.widget') at
>> http:/cdn.jsdelivr.net/jquery.multiselect/1.13/jquery.multiselect.filter.min.js:15
>> http:/cdn.jsdelivr.net/jquery.multiselect/1.13/jquery.multiselect.filter.min.js:15
>> Error: define: Unable to parse JSON string at
>> _SpecRunner.html:21
>> .grunt/grunt-contrib-jasmine/require.js:12 v
>> .grunt/grunt-contrib-jasmine/require.js:19
>> .grunt/grunt-contrib-jasmine/require.js:23
>> .grunt/grunt-contrib-jasmine/require.js:8
>> .grunt/grunt-contrib-jasmine/require.js:23
>> .grunt/grunt-contrib-jasmine/require.js:7 y
>> .grunt/grunt-contrib-jasmine/require.js:23
>> .grunt/grunt-contrib-jasmine/require.js:19
>> .grunt/grunt-contrib-jasmine/require.js:13
>> .grunt/grunt-contrib-jasmine/require.js:7 y
>> .grunt/grunt-contrib-jasmine/require.js:13 E
>> .grunt/grunt-contrib-jasmine/require.js:13
>> .grunt/grunt-contrib-jasmine/require.js:7 y
>> .grunt/grunt-contrib-jasmine/require.js:13 E
>> .grunt/grunt-contrib-jasmine/require.js:13
>> .grunt/grunt-contrib-jasmine/require.js:7 y
>> .grunt/grunt-contrib-jasmine/require.js:13 E
>> .grunt/grunt-contrib-jasmine/require.js:13
>> .grunt/grunt-contrib-jasmine/require.js:7 y
>> .grunt/grunt-contrib-jasmine/require.js:13 E
>> .grunt/grunt-contrib-jasmine/require.js:13
>> .grunt/grunt-contrib-jasmine/require.js:7 y
>> .grunt/grunt-contrib-jasmine/require.js:13 E
>> .grunt/grunt-contrib-jasmine/require.js:13
>> .grunt/grunt-contrib-jasmine/require.js:7 y
>> .grunt/grunt-contrib-jasmine/require.js:13 E
>> .grunt/grunt-contrib-jasmine/require.js:13
>> .grunt/grunt-contrib-jasmine/require.js:7 y
>> .grunt/grunt-contrib-jasmine/require.js:13 E
>> .grunt/grunt-contrib-jasmine/require.js:14
>> .grunt/grunt-contrib-jasmine/require.js:7 y
>> .grunt/grunt-contrib-jasmine/require.js:14 C
>> .grunt/grunt-contrib-jasmine/require.js:28
>> .grunt/grunt-contrib-jasmine/require.js:29

Warning: PhantomJS timed out, possibly due to an unfinished async spec. Use --force to continue.

Aborted due to warnings.
npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0

Moving the file back....

curtstewart@curtis-mbp:StackStudio$ mv config/backend.json.old config/backend.json
curtstewart@curtis-mbp:StackStudio$ npm test

> StackStudio@2.0.0-rc2 test /Users/curtstewart/Git/GitHub/Work/StackStudio
> grunt test

[supressed output]

Running "jasmine:test" (jasmine) task
Testing jasmine specs via phantom
>> TypeError: 'undefined' is not a function (evaluating 'd.widget') at
>> http:/cdn.jsdelivr.net/jquery.multiselect/1.13/jquery.multiselect.min.js:20
>> http:/cdn.jsdelivr.net/jquery.multiselect/1.13/jquery.multiselect.min.js:20
.................................................................
65 specs in 0.365s.
>> 0 failures

Done, without errors.
lifeBCE commented 10 years ago

js/common.js:138

    //Base url for API calls
    var apiUrl = JSON.parse(backendTxt).backend_endpoint;

Anything that requires common.js will require backend.json to be in place.

cstewart87 commented 10 years ago

Maybe the best route here would be to add an appropriate error message here ('config/backend.json Not Found').

In addition to that, I feel like we should stub this call, as well as test that it properly fails when the file doesn't exist.

Any thoughts?

lifeBCE commented 10 years ago

Unit testing in general is not for testing internal implementations especially around installation/configuration. Outside of unit testing, we could add a better message to inform the user that this file is missing (console.info()) but stubbing/mocking it would be an issue (already looked into it). Would have to trap JSON.parse in a spy and that would not be the best thing to do.

Beyond a simple console log message, I am not sure what could be done. Everything requires common.js so are we guaranteed to always have a certain page element loaded into the DOM with which to display in application error message? Even if we did, is this the right place to inform the user of infrastructure issues?

cstewart87 commented 10 years ago

That makes sense. I vote to go ahead and close this issue.

lifeBCE commented 10 years ago

Agreed.