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

Database classes: cover roundtrip fbs+cbor and copy fbs+cbor #29

Open
oberstet opened this issue Mar 26, 2020 · 3 comments
Open

Database classes: cover roundtrip fbs+cbor and copy fbs+cbor #29

oberstet opened this issue Mar 26, 2020 · 3 comments
Assignees
Labels
CI-CD enhancement New feature or request

Comments

@oberstet
Copy link
Contributor

actually, to fully cover the complete interface the database table classes expose, the 2 test functions we currently have per database table are not enough, but we additionally need

  • roundtrip cbor
  • roundtrip fbs
  • copy cbor to fbs
  • copy fbs to cbor

like for example here https://github.com/crossbario/cfxdb/blob/master/cfxdb/tests/test_user.py

@oberstet
Copy link
Contributor Author

note that while all DB classes already have a marshal function (and this function is indeed used in eg the xbrnetwork backend or the market maker), this is not covered by tests, and eg inconsistent types had been a practical headache already

also note, the User.parse class function, which is lacking from most of the other DB classes, but should be there, because it is the "inverse" of marshal

@oberstet oberstet added CI-CD enhancement New feature or request labels Mar 26, 2020
@om26er
Copy link
Contributor

om26er commented Apr 7, 2020

So it seems all our current tests are centered around the fbs variants of things (apart from the tests you linked above) because our database schema classes are from flatbuffers. We'd first need to write schema classes that are cbor-able.

@oberstet do we need to do that now ?

@oberstet
Copy link
Contributor Author

oberstet commented Apr 8, 2020

do we need to do that now ?

all our DB classes need to have "marshal" and "parse", so yes, adding those is part A of this issue. part B is then adding the tests using the new functions (and the existings fbs ones) ..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-CD enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants