Skip to content

Fixes #1870#2527

Merged
JohnMcLear merged 3 commits intoether:developfrom
ldidry:issue-1870-get-saved-revisions-count
Feb 27, 2015
Merged

Fixes #1870#2527
JohnMcLear merged 3 commits intoether:developfrom
ldidry:issue-1870-get-saved-revisions-count

Conversation

@ldidry
Copy link
Copy Markdown
Contributor

@ldidry ldidry commented Feb 24, 2015

Add two functions to API :

  • getSavedRevisionsCount
  • listSavedRevisions

Add two functions to API :
 * getSavedRevisionsCount
 * listSavedRevisions
@JohnMcLear
Copy link
Copy Markdown
Member

Awesome thanks @ldidry can you please include backend tests with this PR? Bump me if you need hlep writing them. They are suppperrrr easy to write though :) J

@ldidry
Copy link
Copy Markdown
Contributor Author

ldidry commented Feb 24, 2015

I'll try.

@ldidry
Copy link
Copy Markdown
Contributor Author

ldidry commented Feb 24, 2015

I got a problem. It's up to you to say if I have to fix it : we can't save revisions through API right now. So if I want to test if the saved revisions count and list vary, I have to add a saveRevision(padID, [rev]) API function. It's pretty easy and I can do that, except for one parameter of addSavedRevision : savedById. I think it's not ok to add an empty parameter or a random string.

Would it be ok for you if I create an author "API" before saving the revision and use it? Or should I just stay with one test for my methods?

Calling saveRevision create an author which name is "API"
These new functions are:
* getSavedRevisionsCount
* listSavedRevisions
* saveRevision

+ typo fixing in backend tests
@ldidry
Copy link
Copy Markdown
Contributor Author

ldidry commented Feb 25, 2015

I didn't wait for your answer, I created a saveRevision API function

@ldidry
Copy link
Copy Markdown
Contributor Author

ldidry commented Feb 27, 2015

ping @JohnMcLear

@JohnMcLear
Copy link
Copy Markdown
Member

Excellent work thanks!

JohnMcLear added a commit that referenced this pull request Feb 27, 2015
@JohnMcLear JohnMcLear merged commit 152f51a into ether:develop Feb 27, 2015
@JohnMcLear
Copy link
Copy Markdown
Member

btw your tests fail

 1) getSavedRevisionsCount gets saved revisions count of Pad:
     Error: Unable to get Saved Revisions Count
      at /home/jose/etherpad-lite/tests/backend/specs/api/pad.js:121:37
      at Test.assert (/home/jose/etherpad-lite/src/node_modules/supertest/lib/test.js:213:13)
      at assert (/home/jose/etherpad-lite/src/node_modules/supertest/lib/test.js:132:12)
      at /home/jose/etherpad-lite/src/node_modules/supertest/lib/test.js:129:5
      at Test.Request.callback (/home/jose/etherpad-lite/src/node_modules/supertest/node_modules/superagent/lib/node/index.js:746:30)
      at Test.<anonymous> (/home/jose/etherpad-lite/src/node_modules/supertest/node_modules/superagent/lib/node/index.js:135:10)
      at Test.emit (events.js:95:17)
      at IncomingMessage.<anonymous> (/home/jose/etherpad-lite/src/node_modules/supertest/node_modules/superagent/lib/node/index.js:938:12)
      at IncomingMessage.emit (events.js:117:20)
      at _stream_readable.js:943:16
      at process._tickCallback (node.js:419:13)

@ldidry
Copy link
Copy Markdown
Contributor Author

ldidry commented Feb 27, 2015

It works on my computer. Strange. Did you run the test or is it continuous integration?

@ldidry
Copy link
Copy Markdown
Contributor Author

ldidry commented Feb 27, 2015

I just tested on the develop branch (with my commit merged): it worked.

@JohnMcLear
Copy link
Copy Markdown
Member

Hrm, try clear your database..

I tried to run the test using bin/backendTests.sh

@ldidry
Copy link
Copy Markdown
Contributor Author

ldidry commented Feb 27, 2015

rm var/dirty.db
 ./bin/safeRun.sh /tmp/t.log
./bin/backendTests.sh

All tests passed well.

@JohnMcLear
Copy link
Copy Markdown
Member

Msut have been a glitch in the matrix, seems fine now :|

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants