Level / memdown

In-memory abstract-leveldown store for Node.js and browsers.
MIT License
287 stars 37 forks source link

Master is failing #50

Closed nolanlawson closed 8 years ago

nolanlawson commented 8 years ago

Looks like a change in abstract-leveldown broke us:

⨯ test put()/get()/del() with `null` value
  not ok 236 should be equal
    ---
      operator: equal
      expected: ''
      actual:   'null'
      at: Immediate.callNext [as _onImmediate] (/Users/nolan/workspace/memdown/memdown.js:177:5)
    ...
⨯ test put()/get()/del() with `undefined` value
  not ok 244 should be equal
    ---
      operator: equal
      expected: ''
      actual:   'undefined'
      at: Immediate.callNext [as _onImmediate] (/Users/nolan/workspace/memdown/memdown.js:177:5)
    ...

I'm having a heck of a time testing this because it doesn't reproduce in the browser, and entirely different tests are broken in the browser for abstract-leveldown.

vweevers commented 8 years ago

The differences between node and browser can probably be explained by (one of) the process.browser if-statements.

nolanlawson commented 8 years ago

It's the _serializeValue in abstract-leveldown AFAICT, yeah.

In the browser, those two tests pass but the ones that fail also fail in abstract-leveldown itself. Unfortunately when I fix them in abstract-leveldown, I run into a browserify error. 😭

nolanlawson commented 8 years ago

Fixed the browser/abstract-leveldown issue (https://github.com/Level/abstract-leveldown/pull/93), but this is still unresolved. Haven't figured it out yet.

nolanlawson commented 8 years ago

If anybody wants to contribute the fix for this issue, it's pretty easy to reproduce:

  1. check out the code
  2. npm it
  3. watch it fail

Alternatively, you can npm run test-browser-local and see the failures live in Zuul using your browser. Unfortunately the tests don't fail in the browser, probably due to a process.browser switch somewhere as stated above.

stockulus commented 8 years ago

created a pull request on abstract-leveldown, that should fix the problem. https://github.com/Level/abstract-leveldown/pull/96

tests are all green after the change

nolanlawson commented 8 years ago

Pinning the version of abstract-leveldown to 2.4.1 also fixes the issue, which I'm perfectly fine with for now.

nolanlawson commented 8 years ago

fixed in #51