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

[hoodie-account-server] replace "pouchdb" with "pouchdb-core", "pouchdb-mapreduce" and "pouchdb-adapter-memory" #59

Closed
9 tasks done
LowProfileDog opened this issue Oct 15, 2016 · 10 comments
Assignees
Labels

Comments

@LowProfileDog
Copy link
Collaborator

LowProfileDog commented Oct 15, 2016

🎃💻👕 Hacktoberfest: Trick or Treat!

If you haven’t yet, sign up for Hacktoberfest to earn an exclusive T-Shirt. Plus I’m sure we can teach you a cool trick or two in the process

🤔 What you will need to know

You should have worked with JavaScript, Node.js and Testing. If you haven’t yet, we recommend the JavaScript Track on exercism.io

hoodie-account-server currently has PouchDB in its devDependencies. We need PouchDB for testing.

But pouchdb comes with all sorts of features that we don’t need, but they slow down the installation time and also install native dependencies that are sometimes hard to setup, especially on older windows machines.

Luckily, our friends at PouchDB created custom builds. That allows us to only pick exactly the features that we need, which are

🎯 The Goal

Minimise the installation time and requirements by replacing the pouchdb package with the packages listed above.

📋 Step by Step

If this is your first, welcome 🎉 😄 Here is a great tutorial on how to send a pull request using the terminal.

  • 🙋 Claim this issue: Comment below (or assign yourself and continue at step 4 :)
    Please 🙏 only claim if you want to start working on it during the event.
    Once claimed we add you as contributor to this repository.
  • 👌 Accept our invitation to this repository. Once accepted, assign yourself to this repository
  • 👓 Please review our Code of Conduct
    In a nutshell: be patient and actively kind with us 😊
  • 🔄 replace the up for grabs label with in progress.
  • 🗜 Setup the repository locally make sure that all tests pass
  • replace pouchdb with the other packages in package.json and tests/integration/utils/get-server.js#L6
  • Commit the change with test: replace pouchdb with custom build for testing and push it either to your fork or push your new branch.
  • 🔀 Start a Pull Request. Mention closes hoodiehq/camp#59 in the description.
  • 🏁 Done 👍 Replace the in progress label with ready. Ask in comments for a review :)

🤔❓ Questions

Ping us in the Hoodie Chat or on Twitter

@nistath
Copy link
Collaborator

nistath commented Oct 20, 2016

I can work on this!

@nistath
Copy link
Collaborator

nistath commented Oct 20, 2016

I wrote the code required a while ago, but the testing process is kind of confusing, as it gets stuck at the Reading TAP data from stdin (use "-" argument to suppress) part, where all it does is mirror what I write in stdin.

@LowProfileDog
Copy link
Collaborator Author

Thanks for looking into it @nistath The issue is all yours :) I’ve invited you to our Camp repository, you can accept my invitation at https://github.com/hoodiehq/camp/invitations

Once accepted, you can assign yourself to this issue and check off the checkboxes above. Enjoy 🐕

Can you tell us your node & npm version? Run npm -v and node -v

@nistath nistath self-assigned this Oct 22, 2016
@nistath
Copy link
Collaborator

nistath commented Oct 22, 2016

@LowProfileDog Here's what happens when I run the test.

$ npm test

@hoodie/account-server@0.0.0-semantically-released pretest C:\Users\Nick\Deskt op\hoodie\hoodie-account-server
standard

@hoodie/account-server@0.0.0-semantically-released test C:\Users\Nick\Desktop\ hoodie\hoodie-account-server
nyc tap 'tests/{unit,integration}/*/-test.js'

Reading TAP data from stdin (use "-" argument to suppress)
asdasdasdas
asdasdasdas

and it keeps mirroring my input forever.

@nistath
Copy link
Collaborator

nistath commented Oct 22, 2016

The problem is that this works even if pouchdb is not installed. I have no idea if my new code works or not, although it is written according to the pouchdb documentation.

@nistath
Copy link
Collaborator

nistath commented Oct 22, 2016

I'm on node 6.5.0 and npm 3.10.3 on Windows 8.

@gr2m
Copy link
Member

gr2m commented Oct 22, 2016

@nistath I don’t know why the test is behaving that way for you yet, I’ve never seen that behavior before.

One thinking mistake I found is that we don’t want the memory adapter, instead we want the http adapter, otherwise PouchDB won’t send requests and our CouchDB mocks will stop working

You can also try to run a single test. I’m not sure if it’s the same on Windows, but on mac you can execute the binary from the node_modules folder directly, for example

./node_modules/.bin/tap tests/integration/routes/no-cookie-header-test.js

Will only run the tests defined in tests/integration/routes/no-cookie-header-test.js

Can you check if that helps in any way?

@gr2m
Copy link
Member

gr2m commented Oct 22, 2016

yeah fix was simple, I updated your PR: hoodiehq/hoodie-account-server#213 and will merge it once CI passes, thanks :)

@nistath
Copy link
Collaborator

nistath commented Oct 23, 2016

That specific test ran successfully. I think there's one test that does that, and goes in an infinite loop not letting the other tests run. Either that, or TAP has some issues.

@gr2m
Copy link
Member

gr2m commented Oct 23, 2016

all good, fixed via hoodiehq/hoodie-account-server#213

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants