Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Binary data and json test #1599

Merged
merged 2 commits into from
Aug 25, 2015
Merged

Binary data and json test #1599

merged 2 commits into from
Aug 25, 2015

Conversation

chriscool
Copy link
Contributor

This is a test for issue #1582 (JSON encoding corrupts non-UTF-8 binary data).

But please don't merge this until the issue of the license of UTF-8-test.txt is fixed.

The first patch adds a file that is CC BY 4.0 licensed, but after discussing with the author I think it is ok.

@jbenet jbenet added the status/in-progress In progress label Aug 23, 2015
@chriscool chriscool mentioned this pull request Aug 23, 2015
Closed
@chriscool
Copy link
Contributor Author

Go tests are failing which is not related.

@chriscool chriscool mentioned this pull request Aug 24, 2015
Closed
16 tasks
@GitCop
Copy link

GitCop commented Aug 24, 2015

There were the following issues with your Pull Request

  • Commit: 241309c
    • Invalid signoff. Commit message must end with
      License: MIT
      Signed-off-by: .* <.*>

Guidelines and a script are available to help. Your feedback on GitCop is welcome on this issue.

This message was auto-generated by https://gitcop.com

@chriscool
t0051: add UTF-8 test file
e4a0a3b 
This new file comes from:

https://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt

It is licensed under CC BY 4.0.

After discussing that with its author, I think it is ok to
add it as is to our repository. The only restriction might
be that we should indicate in the file any change we make
to it.

License: CC BY 4.0
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
@GitCop
Copy link

GitCop commented Aug 24, 2015

There were the following issues with your Pull Request

  • Commit: e4a0a3b
    • Invalid signoff. Commit message must end with
      License: MIT
      Signed-off-by: .* <.*>

Guidelines and a script are available to help. Your feedback on GitCop is welcome on this issue.

This message was auto-generated by https://gitcop.com

@chriscool
t0051: check json i/o
5cced6f 
License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
@GitCop
Copy link

GitCop commented Aug 24, 2015

There were the following issues with your Pull Request

  • Commit: e4a0a3b
    • Invalid signoff. Commit message must end with
      License: MIT
      Signed-off-by: .* <.*>

Guidelines and a script are available to help. Your feedback on GitCop is welcome on this issue.

This message was auto-generated by https://gitcop.com

@chriscool chriscool changed the title [DO NOT MERGE] Binary data and json Binary data and json test Aug 24, 2015
@chriscool
Copy link
Contributor Author

Ok, I think it should be ok to merge this now.

@ghost ghost mentioned this pull request Aug 24, 2015
Closed
56 tasks
@jbenet
Copy link
Member

jbenet commented Aug 25, 2015

@chriscool so it's not broken?

cc @davidar

@chriscool
Copy link
Contributor Author

Yes it is broken. The last test uses test_expect_failure:

+   test_expect_failure "'ipfs object put --inputenc=json' output looks good" '
+       echo "added $HASH" >expected &&
+       test_cmp expected actual
+   '

You can also wait for someone to fix the problem, but I think it will be valuable to add some tests for this anyway.

@jbenet
Copy link
Member

jbenet commented Aug 25, 2015

@chriscool ok sounds good.

jbenet added a commit that referenced this pull request Aug 25, 2015
@jbenet
Merge pull request #1599 from ipfs/binary-data-and-json
0f90659 
Binary data and json test
@jbenet jbenet merged commit 0f90659 into master Aug 25, 2015
@jbenet jbenet removed the status/in-progress In progress label Aug 25, 2015
@jbenet jbenet deleted the binary-data-and-json branch August 25, 2015 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chriscool @GitCop @jbenet