Wizcorp / node-pivotal

NodeJS API library for PivotalTracker
44 stars 20 forks source link

Fixed tests, added filters for group-specific getIteration methods, added getProjectActivities method #8

Closed tiii closed 11 years ago

tiii commented 11 years ago

Hey, I added filtering for limit and offset to the group-specific getIterarion methods.

Furthermore i fixed the tests and cleaned up (a bit) of the mess in the documentation. (in code & README.md since i cant run pcregrep on osx, so you may want to run make doc and check the output)

Btw: at npm theres an outdated version of this package which doesn't include group-specific methods at all

stelcheck commented 11 years ago

Sorry for the late response.

When I run the test I get:

Calling getProject

events.js:68
        throw new Error("Uncaught, unspecified 'error' event.");
              ^
Error: Uncaught, unspecified 'error' event.
    at Parser.EventEmitter.emit (events.js:68:15)
    at Parser.exports.Parser.Parser.parseString (/home/mt/node-pivotal/node_modules/xml2js/lib/xml2js.js:223:21)
    at Parser.__bind [as parseString] (/home/mt/node-pivotal/node_modules/xml2js/lib/xml2js.js:6:61)
    at IncomingMessage.pivotal.apiCall.req.on.cb.errors.error (/home/mt/node-pivotal/index.js:767:20)
    at IncomingMessage.EventEmitter.emit (events.js:115:20)
    at IncomingMessage._emitEnd (http.js:366:10)
    at HTTPParser.parserOnMessageComplete [as onMessageComplete] (http.js:149:23)
    at CleartextStream.socketOnData [as ondata] (http.js:1356:20)
    at CleartextStream.CryptoStream._push (tls.js:485:27)
    at SecurePair.cycle (tls.js:839:20)

Do you get any errors on your end?

stelcheck commented 11 years ago

Seems like the behavior is different between 0.4.12 and, say, 0.8 (see last message), but the outcome is the same. In fact, 0n 0.4.12 you have proper, expected log output while you get an uncaught exception on 0.8.x....

stelcheck commented 11 years ago

getDoneIteration looks also broken:

Calling getDoneIterations (limit:3) 519957
{ limit: 3, group: 'done' }
requesting /services/v3/projects/519957/iterations/done?limit=3 GET null
info Result: <?xml version="1.0" encoding="UTF-8"?>
<nil-classes type="array"/>

info Result: <?xml version="1.0" encoding="UTF-8"?>
<nil-classes type="array"/>

error Result: <?xml version="1.0" encoding="UTF-8"?>
<nil-classes type="array"/>

Error {"errors":{"error":["Error while parsing PT HTTP service response","Cannot read property 'number' of undefined"]}}
stelcheck commented 11 years ago

According to documentation

For done iterations, offset should be negative, relative to the most recent done iteration. The following examples retrieves the last 6 done iterations:
tiii commented 11 years ago

Okay - thanks for your feedback. I included it in the latest commits.

Sorry the test only worked if one iteration was returned (which was true in my case). Maybe we should consider always returning arrays in methods where multiple results are expected - even if there is only one result? (e.g. getProjects, getMemberships, getIterations...)

stelcheck commented 11 years ago

Yes, I think that would make a lot of sense.

Is still have some failure with the getDoneIterations, but that might be because the result is this

<?xml version="1.0" encoding="UTF-8"?>
<nil-classes type="array"/>

Which might not be handled properly either by the XML parser or some of the node-pivotal code:

events.js:68
        throw new Error("Uncaught, unspecified 'error' event.");
              ^
Error: Uncaught, unspecified 'error' event.
    at Parser.EventEmitter.emit (events.js:68:15)
    at Parser.exports.Parser.Parser.parseString (/home/mt/node-pivotal/node_modules/xml2js/lib/xml2js.js:223:21)
    at Parser.__bind [as parseString] (/home/mt/node-pivotal/node_modules/xml2js/lib/xml2js.js:6:61)
    at IncomingMessage.pivotal.apiCall.req.on.cb.errors.error (/home/mt/node-pivotal/index.js:779:20)
    at IncomingMessage.EventEmitter.emit (events.js:115:20)
    at IncomingMessage._emitEnd (http.js:366:10)
    at HTTPParser.parserOnMessageComplete [as onMessageComplete] (http.js:149:23)
    at CleartextStream.socketOnData [as ondata] (http.js:1356:20)
    at CleartextStream.CryptoStream._push (tls.js:485:27)
    at SecurePair.cycle (tls.js:839:20)

Ill try to add a done iteration to my test project and move on from there

tiii commented 11 years ago

We should mock the api-responses. Nevertheless this is an error which is likely to happen in all requests made by node-pivotal that return no data and should be handled.

stelcheck commented 11 years ago

Merged your code through a different commit, added you as a commiter. Checkout the current master, let me know if that is good enough for you.