refactor: Remove ixml refs in SqlCall#102
refactor: Remove ixml refs in SqlCall#102kadler merged 2 commits intoIBM:masterfrom abmusse:remove-ixml-for-sqlcall
Conversation
|
After removing ixml refs from $ npm test test/unit/SqlCallUnit.js
....
47 passing (30ms)
3 failing
1) SqlCall Class Unit Tests
connect
appends connect to sql XML:
TypeError: sql.connect is not a function
at Context.<anonymous> (test/unit/SqlCallUnit.js:190:11)
at processImmediate (internal/timers.js:439:21)
2) SqlCall Class Unit Tests
setOptions
appends options to sql XML:
TypeError: sql.setOptions is not a function
at Context.<anonymous> (test/unit/SqlCallUnit.js:200:11)
at processImmediate (internal/timers.js:439:21)
3) SqlCall Class Unit Tests
setOptions
appends options without options object to sql XML:
TypeError: sql.setOptions is not a function
at Context.<anonymous> (test/unit/SqlCallUnit.js:209:11)
at processImmediate (internal/timers.js:439:21)All passed except These functions were removed with #56 and these unit tests also need to be removed. |
|
Opened #106 for the above issue |
lib/SqlCall.js
Outdated
| addQuery(stmt, options) { | ||
| if (options && options.error) { | ||
| this.xml += iXml.iXmlNodeSqlQueryOpen(options.error) + stmt + iXml.iXmlNodeSqlQueryClose(); | ||
| this.xml += `<query error='${options.error}'>${stmt}</query>`; | ||
| } else { | ||
| this.xml += iXml.iXmlNodeSqlQueryOpen() + stmt + iXml.iXmlNodeSqlQueryClose(); | ||
| this.xml += `<query error='fast'>${stmt}</query>`; | ||
| } | ||
| } |
There was a problem hiding this comment.
Most of these could be something like
addQuery(stmt, options={}) {
this.xml += `<prepare error='${options.error || 'fast'}'>${stmt}</prepare>`;
}
|
Noticed that SqlCall.commit will no-op when options.action is not specified. Check out the unit test: https://github.com/IBM/nodejs-itoolkit/blob/master/test/unit/SqlCallUnit.js#L113 Looks like iXmlNodeSqlCommit intended to default to I think this should default to |
lib/SqlCall.js
Outdated
| } | ||
|
|
||
| tables(params, options = {}) { | ||
| this.xml += `<tables${options.error ? ` error='${options.error}'` : ''}>`; |
There was a problem hiding this comment.
| this.xml += `<tables${options.error ? ` error='${options.error}'` : ''}>`; | |
| this.xml += `<tables error='${options.error || 'fast'}'>`; |
It's a slight behavior change, but much clearer.
There was a problem hiding this comment.
And by behavior change, I mean that we would be extraneously sending the default error option instead of leaving it blank and letting XMLSERVICE default it. The default is still the same, however, so the only difference is that we send some extra data in the XML that we didn't need to, but it's unlikely to hurt anything.
There was a problem hiding this comment.
Makes sense we would need to update the unit tests as well to expect error to be sent by default now
lib/SqlCall.js
Outdated
| this.xml += iXml.iXmlNodeSqlTableprivOpen(); | ||
| } | ||
| tablePriv(params, options = {}) { | ||
| this.xml += `<tablepriv${options.error ? ` error='${options.error}'` : ''}>`; |
lib/SqlCall.js
Outdated
| this.xml += iXml.iXmlNodeSqlSpecialOpen(); | ||
| } | ||
| special(params, options = {}) { | ||
| this.xml += `<special${options.error ? ` error='${options.error}'` : ''}>`; |
There was a problem hiding this comment.
Looks like there's a bunch of them. All of them should be adjusted.
kadler
left a comment
kadler
left a comment
There was a problem hiding this comment.
One minor change and I think we're good!
- Add error=fast default values for tests - Add action=rollback default for commit tests
2 participants
Part of #96
Resolves #110