adlnet / lrs-conformance-test-suite

A NodeJS project that tests the MUST requirements of the xAPI Spec and is based on the ADL testing requirements repository. The test suite website can be found here: https://lrstest.adlnet.gov/. The adopters website can be found here: https://adopters.adlnet.gov/
https://adlnet.gov/projects/xapi/
MIT License
67 stars 42 forks source link

superagent: double callback bug occurs #219

Closed n-crossfire-shibata closed 7 years ago

n-crossfire-shibata commented 7 years ago

Hi, I'm developing our new LRS.

I passed most tests.

{"total":1359,"passed":1358,"failed":1,"version":"1.0.3.15"}

But I encountered an inexplicable error and I couldn't solve it.

Situation

Only one test couldn’t be passed, that one is here.

https://github.com/adlnet/lrs-conformance-test-suite/blob/master/test/v1_0_3/H.Communication1.3-AlternateRequestSyntax.js#L111

Here’s my command.

node bin/console_runner.js -e http://localhost:3000/v1/ -a -u user -p password -g "will fail PUT with no content body"

Our LRS seems return response with HTTP status code 400 (and of course, return version status in response header).

Here’s command logs.

runTests starting
Grep is /will fail PUT with no content body/
directory is  [ 'v1_0_3' ]
Setting up
Accounting for time differential between test suite and lrs
superagent: double callback bug. Upgrade to v3.2+ to fix this
{"total":1,"passed":0,"failed":1,"version":"1.0.3.15"}
Tests completed in 0.38 seconds
Test Suite Complete
Full run log written to /path/to/lrs-conformance-test-suite/logs/ab080319-6789-4c59-a570-572d213df652.log
Closed

Problem

Maybe crashed by superagent: double callback bug. Upgrade to v3.2+ to fix this.

Here’s full log of execute.

ab080319-6789-4c59-a570-572d213df652.log

The "will fail PUT with no content body” test has blank log. I think it is problem of your test suite, or did I make something wrong?

Thanks.

cr8onski commented 7 years ago

Thanks for bringing this to our attention. I just noticed that the version of the test suite used is 1.0.3.15. The current version of the test suite is 1.0.3.17. Would you please run this test against the latest version of the test suite and see if the error still occurs?

n-crossfire-shibata commented 7 years ago

Thanks for replying. I tried git pull and npm update and do node bin/console_runner.js -e http://localhost:3000/v1/ -a -u user -p password -g "will fail PUT with no content body" again.

But unfortunately, seems no change in target file.

git pull
remote: Counting objects: 66, done.
remote: Total 66 (delta 45), reused 45 (delta 45), pack-reused 21
Unpacking objects: 100% (66/66), done.
From github.com:adlnet/lrs-conformance-test-suite
   81d09e2..3a04557  master     -> origin/master
   dc47cf4..e4816e1  development -> origin/development
 * [new branch]      simplerfix -> origin/simplerfix
   81d09e2..3e1548f  staged     -> origin/staged
Updating 81d09e2..3a04557
Fast-forward
 bin/console_runner.js                               |  9 +++++----
 test/v1_0_3/H.Communication2.1-StatementResource.js | 18 +++++++++---------
 test/v1_0_3/H.Communication2.3-StateResource.js     | 21 +++++++++++++++------
 .../H.Communication2.6-AgentProfileResource.js      |  6 ++++--
 .../H.Communication2.7-ActivityProfileResource.js   | 10 ++++++++--
 version.js                                          |  2 +-
 6 files changed, 42 insertions(+), 24 deletions(-)

And it still fails.

Here's full log.

267c6462-1f5e-43d1-8cef-2ff1d987a68b.log

Do you have any ideas?

note

This requires npm for installation. NodeJS version 4.x or later.

I have also changed version of node. Both 4.4.7 and 4.8.4 have failed.

cr8onski commented 7 years ago

In the logs, it has nothing where the request should be. Is that true? If so, it seems to be hitting an error before the initial request can happen.

n-crossfire-shibata commented 7 years ago

OK, I take full test with node bin/console_runner.js -e http://localhost:3000/v1/ -a -u username -p password command (without -g option).

Here's full log.

e95a776a-8c22-4bbe-a3e5-432dc11b408f.log

I think that you can see only one part is strange.

Is it enough to investigate? If you want more informations, please tell me again.

Thanks.

creighton commented 7 years ago

I just tried this on my machine and it passed. Can you try node bin/console_runner.js -e https://lrs.adlnet.gov/xapi -a -u tom -p 1234 -g "will fail PUT with no content body"? That will run your installation of the test suite against our LRS.

n-crossfire-shibata commented 7 years ago

@creighton Thanks for replying.

Can you try node bin/console_runner.js -e https://lrs.adlnet.gov/xapi -a -u tom -p 1234 -g "will fail PUT with no content body"?

I tried this, and it passed. So I have some questions.

In our application log, it seems response with 400 (We use Ruby on Rails in development).

Started POST "/v1/statements?method=PUT" for 127.0.0.1 at 2017-10-25 09:50:40 +0900
Processing by StatementsController#create as JSON
  Parameters: {"method"=>"PUT"}
Can't verify CSRF token authenticity.
  User Load (0.3ms)  SELECT  `users`.* FROM `users` WHERE `users`.`auth_user` = 'XXXX' LIMIT 1
  Rendering text template
  Rendered text template (0.0ms)
Completed 400 Bad Request in 3ms (Views: 0.7ms | ActiveRecord: 0.3ms | Elasticsearch: 0.0ms)

First of all, I try to reproduce same request in correct log. Thanks.

n-crossfire-shibata commented 7 years ago

Finally, I found unintended double rendering by Ruby on Rails itself. Maybe it causes problem, and I fixed it.

{"total":1359,"passed":1359,"failed":0,"version":"1.0.3.17"}

I passed all tests in my local machine.

We appreciate your cooperation. 😄

creighton commented 7 years ago

Glad you found it! Congratulations on passing the test suite. Hopefully all will pass running the LRS Test Suite and we will see you on the xAPI Adopters site.