fix: change test to read string, not buffer #2042

2020-07-03T00:04:57Z

wlawt commented

2020-07-03 00:04:57 +00:00

(Migrated from github.com)

The browser test CSV > input > should interpret CRLF newlines was throwing an error for buffer not being defined.

A quick fix was to read it as a string instead of a buffer.

This also passes the browser tests and the Sauce Labs tests.

The browser test `CSV > input > should interpret CRLF newlines` was throwing an error for buffer not being defined. A quick fix was to read it as a `string` instead of a `buffer`. This also passes the browser tests and the Sauce Labs tests.

SheetJSDev (Migrated from github.com) approved these changes 2020-07-03 00:08:21 +00:00

mohammedsahl (Migrated from github.com) approved these changes 2020-07-03 00:18:02 +00:00

mohammedsahl (Migrated from github.com) left a comment

LGTM. Has this been an issue for some time or did it just pop up recently? Just curious

SheetJSDev commented

2020-07-03 00:21:52 +00:00

(Migrated from github.com)

Relatively recently. When we originally made the test for nodejs, Buffer was used. That's not available in browser. @wlawt found that the some of the node tests weren't be run in-browser, and tried reconciling the two test suites in a previous commit. He tried to run the browser tests and found this issue, and the PR addresses the problem (https://app.saucelabs.com/builds/c1670f5dd3384c968095e8ed797c3d57 is the browser test run)

👍 2

Sign in to join this conversation.

No reviewers

SheetJSDev

mohammedsahl