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

db.initValue (integration tests failing due to connection to seed nodes) #331

Closed
moshababo opened this issue Aug 12, 2018 · 8 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@moshababo
Copy link
Collaborator

WebProxy.spec.ts is failing due to connection to seed nodes, which cause an error on xud.shutdown.
We should avoid making xud instance always connect to the seed nodes.

@moshababo moshababo added the bug Something isn't working label Aug 12, 2018
@sangaman
Copy link
Collaborator

I agree, I haven't seen that happen to me yet but I'd raised this as a possibility earlier. What's the error you're seeing on shutdown tho?

@moshababo
Copy link
Collaborator Author

Something to do with the db connection, probably when fetching entries for packet a response. Don't see it at the moment because my connections are failing due to blocked ports (i'm working from a library recently). When connections failing, xud.shutdown just hangs, and tests are not completing.

Proposal:

  • add db.initValues config property, default to true. if true, when db is created, db.initValues method will be called. This will inject the seed hosts values, and other necessary values (currencies/pairs, for now). It could be used for the test db as well, if needed. If we want a different set of values for that (for example, injecting currencies/pairs without seed hosts), we can just create another method, and execute it instead.
  • these methods can be made as tasks, to be removed from the DB class (not mandatory)
  • db:init npm script will be removed, since it's not in usage (correct me if i'm wrong). @kilrau

This blocks #96 completion as well.

@kilrau
Copy link
Contributor

kilrau commented Aug 13, 2018

EDIT: we'll go with Moshe's approach as above. Get rid of db:init, no xud_test db anymore, just one xud db gets initialized with the 4 default currencies and 2 default pairs on creation and that's it.

@kilrau kilrau added this to the 1.0.0-alpha milestone Aug 14, 2018
@kilrau kilrau modified the milestones: 1.0.0-alpha, 1.0.0-prealpha.2 Aug 14, 2018
@kilrau kilrau changed the title integration tests failing due to connection to seed nodes db.initValue (integration tests failing due to connection to seed nodes) Aug 14, 2018
@kilrau kilrau added the to do label Aug 14, 2018
@sangaman
Copy link
Collaborator

I'm a bit confused, the test db will still be used for running automated tests, right? And which 2 pairs/4 currencies are we adding here? I was thinking just btc/ltc.

@moshababo
Copy link
Collaborator Author

test db has a different config section, so it could still be used whenever needed.

values: just copy everything from tasks/db/init.ts, or rather just run the task instead.

@kilrau
Copy link
Contributor

kilrau commented Aug 15, 2018

the test db will still be used for running automated tests, right?
test db has a different config section, so it could still be used whenever needed.

You know better if you use the testDB for something or not. If it's not used for anything, I suggest to remove if from init, if it's used, keep it.

And which 2 pairs/4 currencies are we adding here?

  await orderBookRepository.addCurrencies([
    { id: 'BTC' },
    { id: 'LTC' },
    { id: 'ZRX' },
    { id: 'GNT' },
  ]);

  await Promise.all([,
    orderBookRepository.addPairs([
      { baseCurrency: 'BTC', quoteCurrency: 'LTC', swapProtocol: SwapProtocol.LND },
      { baseCurrency: 'ZRX', quoteCurrency: 'GNT', swapProtocol: SwapProtocol.RAIDEN },
    ]),
  ]);

To have one pair for LN and one pair for Raiden. Then we don't have to touch this again once Raiden is working.

@moshababo
Copy link
Collaborator Author

moshababo commented Aug 15, 2018

test db config is used for creating a temp database in test automation.

in regards to the actual tasks files - i'll adjust them after this issue will be solved.

@kilrau
Copy link
Contributor

kilrau commented Aug 15, 2018

test db config is used for creating a temp database in test automation.

Then keep it. I didn't know that.

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

No branches or pull requests

3 participants