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.

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

Conformance tests for JavaScript (Node.js). 15 tests are failing. #1662

Merged
merged 5 commits into from
Mar 27, 2017

Conversation

haberman
Copy link
Member

@haberman haberman commented Jun 8, 2016

Review to @joeltine.

@haberman
Copy link
Member Author

retest this please


/*
* Protocol Buffers - Google's data interchange format
* Copyright 2008 Google Inc. All rights reserved.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2016?

@joeltine
Copy link

@haberman high level comment: can you please provide more context on what this change is trying to accomplish? I'm going into this without any background and a summary would help. A top-level comment in the conformance test wouldn't be a bad idea.

@haberman haberman changed the title Conformance tests for JavaScript (Node.js). All tests pass! Conformance tests for JavaScript (Node.js). 15 tests are failing. Mar 13, 2017
@haberman haberman requested a review from acozzette March 15, 2017 17:30
function readBuffer(bytes) {
var buf = new Buffer(bytes);
var totalRead = 0;
//console.warn("Want to read: " + bytes);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to delete these commented out console.warn statements.

@acozzette
Copy link
Member

Could you perhaps add some short instructions to conformance/README.md about how to run the new tests? Otherwise LGTM!

@haberman
Copy link
Member Author

Addressed your comments, PTAL. Thanks!

@acozzette
Copy link
Member

LGTM

@haberman haberman merged commit c565e25 into protocolbuffers:master Mar 27, 2017
@haberman haberman deleted the jsconformance branch January 2, 2019 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants