You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by radekg <gi...@git.apache.org> on 2015/01/02 19:26:42 UTC
[GitHub] thrift pull request: THRIFT-2926: JS: Binary protocol with unit te...
GitHub user radekg opened a pull request:
https://github.com/apache/thrift/pull/345
THRIFT-2926: JS: Binary protocol with unit tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/radekg/thrift js-binary-protocol
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/345.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #345
----
commit 14f0f6b1a3989af01fea1a1de44f5d602665e719
Author: radekg <ra...@gruchalski.com>
Date: 2015-01-02T16:12:45Z
JS: Binary protocol with unit tests.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2926: JS: Binary protocol with unit te...
Posted by radekg <gi...@git.apache.org>.
Github user radekg commented on the pull request:
https://github.com/apache/thrift/pull/345#issuecomment-69494149
On it and a broader discussion here: https://issues.apache.org/jira/browse/THRIFT-2926
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2926: JS: Binary protocol with unit te...
Posted by radekg <gi...@git.apache.org>.
Github user radekg commented on the pull request:
https://github.com/apache/thrift/pull/345#issuecomment-68589507
Thank you. I'll take a look at this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #345: THRIFT-2926: JS: Binary protocol with unit tests.
Posted by nsuke <gi...@git.apache.org>.
Github user nsuke commented on the issue:
https://github.com/apache/thrift/pull/345
@ebremer none that I'm not aware of.
You can use lib/nodejs code with browserify (or probably webpack), though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2926: JS: Binary protocol with unit te...
Posted by radekg <gi...@git.apache.org>.
Github user radekg commented on the pull request:
https://github.com/apache/thrift/pull/345#issuecomment-68693024
@bufferoverflow I am trying to understand where the problem is. It seems that travis is using a different method of testing than the one documented in the README for js tests. The README assumes grunt as a main way of testing while travis seems to be using ant?
I am also trying to figure out how does one run the main set of tests. Do I have to follow this write up https://thrift.apache.org/docs/BuildingFromSource ?
Regarding the failing tests, travis complains about some invalid bitwise operations while bitwise is allowed by build.xml jslint. Not sure where to start looking for stuff.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2926: JS: Binary protocol with unit te...
Posted by radekg <gi...@git.apache.org>.
Github user radekg commented on the pull request:
https://github.com/apache/thrift/pull/345#issuecomment-68764557
And the code: https://github.com/radekg/thrift-js-binary-protocol
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2926: JS: Binary protocol with unit te...
Posted by radekg <gi...@git.apache.org>.
Github user radekg closed the pull request at:
https://github.com/apache/thrift/pull/345
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift issue #345: THRIFT-2926: JS: Binary protocol with unit tests.
Posted by jeking3 <gi...@git.apache.org>.
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/345
This pull request is being closed due to lack of activity.
You can re-open it after rebasing on the current master if you would like it to be reconsidered.
---
[GitHub] thrift issue #345: THRIFT-2926: JS: Binary protocol with unit tests.
Posted by ebremer <gi...@git.apache.org>.
Github user ebremer commented on the issue:
https://github.com/apache/thrift/pull/345
Is there any further work being done to support TBinaryProtocol and TCompact in Javascript?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request #345: THRIFT-2926: JS: Binary protocol with unit tests.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/345
---
[GitHub] thrift pull request: THRIFT-2926: JS: Binary protocol with unit te...
Posted by radekg <gi...@git.apache.org>.
GitHub user radekg reopened a pull request:
https://github.com/apache/thrift/pull/345
THRIFT-2926: JS: Binary protocol with unit tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/radekg/thrift js-binary-protocol
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/345.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #345
----
commit e79c300f09eb781bee5a2cc402bea0eb370ca2a4
Author: radekg <ra...@gruchalski.com>
Date: 2015-01-02T16:12:45Z
JS: Binary protocol with unit tests.
commit e74c0d7c40fe3de9312ee571990fcbac8fa28249
Author: radekg <ra...@gruchalski.com>
Date: 2015-01-05T19:28:23Z
Integrated TBinaryProtocol into grunt tests to present the PhantonJS issue.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2926: JS: Binary protocol with unit te...
Posted by bufferoverflow <gi...@git.apache.org>.
Github user bufferoverflow commented on the pull request:
https://github.com/apache/thrift/pull/345#issuecomment-69494103
Thanks @radekg for this udate.
any idea about the jslint issues?
cd lib/js/test
make check
lib/js/test/build.xml:216: JSLint: 39 errors in 1 file
They might be related to the unit test issues:
```
cd lib/js/test
ant unittest
...
unittest:
[echo] Running Unit Tests with headless browser!
[exec] Xlib: extension "RANDR" missing on display ":99".
[java] Timeout: killed the sub-process
[java] Java Result: -1
[exec] 'waitFor()' finished in 403ms.
[exec] Tests completed in 267 milliseconds.
[exec] 4 assertions of 41 passed, 37 failed.
```
You can also remove ThriftTest.thrift file from this commit it is a duplication.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2926: JS: Binary protocol with unit te...
Posted by radekg <gi...@git.apache.org>.
Github user radekg commented on the pull request:
https://github.com/apache/thrift/pull/345#issuecomment-68580492
This build has failed because of some jslint issues. Any hints how can I help resolving?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2926: JS: Binary protocol with unit te...
Posted by bufferoverflow <gi...@git.apache.org>.
Github user bufferoverflow commented on the pull request:
https://github.com/apache/thrift/pull/345#issuecomment-68587476
jslint is used via lib/js/test/build.xml which also runs the unit tests test.js/test.html
My favorite would be integration of your tests into test.js/test.html and use guessProtocolFactory within the server part https://github.com/apache/thrift/blob/master/lib/js/test/src/test/Httpd.java#L217
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] thrift pull request: THRIFT-2926: JS: Binary protocol with unit te...
Posted by radekg <gi...@git.apache.org>.
Github user radekg commented on the pull request:
https://github.com/apache/thrift/pull/345#issuecomment-68758030
I've done a little bit more research into this. There's a major issue which will surface in the unit test once I can get it integrated. I've got the test properly integrated in the grunt task. What happens it that PhantomJS doesn't like the following expression:
String.fromCharCode.apply(null, new Uint8Array( buffer ) )
This has been described here: https://github.com/mozilla/pdf.js/issues/1955
The problem is, if I apply the "fix" mentioned in the comments, the test doesn't pass because unicode characters are decoded incorrectly. So, one way or another, the test will fail. I can see only one solution to this issue. I am going to close this pull request and provide the ThrifBinaryProtocol for JS as an external module to be added on top of Thrift library. Once this is done, I will post a link to the project here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---