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

Master is failing #50

Closed
nolanlawson opened this issue May 30, 2016 · 7 comments
Closed

Master is failing #50

nolanlawson opened this issue May 30, 2016 · 7 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@nolanlawson
Copy link
Contributor

nolanlawson commented May 30, 2016

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.

@nolanlawson nolanlawson added bug Something isn't working help wanted Extra attention is needed has test case labels May 30, 2016
@vweevers
Copy link
Member

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

@nolanlawson
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@nolanlawson
Copy link
Contributor Author

nolanlawson commented May 30, 2016

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
Copy link

stockulus commented May 30, 2016

created a pull request on abstract-leveldown, that should fix the problem.
Level/abstract-leveldown#96

tests are all green after the change

@nolanlawson
Copy link
Contributor Author

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

@nolanlawson
Copy link
Contributor Author

fixed in #51

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants