search

DAVFoundation / dav-js

Enable integration of JavaScript, TypeScript, and Node.js code with the DAV Network
https://developers.dav.network/
MIT License
75 stars 51 forks source link

Create tests for `vessel-charging/BidParams` using jest. #18

Closed haialaluf closed 5 years ago

haialaluf commented 5 years ago

first-timers-only

This issue is tagged :octocat: first-timers-only. It is only for people who have never contributed to open source before, and are looking for an easy way take their first steps.

Consider this your chance to dip your toe into the world of open-source, and get some bragging rights for writing code that makes drones fly, lets cars find charging stations, helps people and goods get from place to place, and more.

Find more first-timers-only issues from DAV Foundation here.

Thank you for your help :heart:

What is this project?

DAV (Decentralized Autonomous Vehicles) is a new foundation working to build an open-source infrastructure for autonomous vehicles (cars, drones, trucks, robots, and all the service providers around them) to communicate and transact with each other over blockchain.

As an organization that believes in building a large community of open-source contributors, we often create issues like this one to help people take their first few steps into the world of open source.

dav-js

This SDK enabled integrating any client side JS and NodeJS code with the DAV Network.

How you can help

The Issue

Create tests for vessel-charging/BidParams using jest.

You need to create tests to check that the serialize and deserialize methods work as expected.

Please use the test file for class NeedParams as a basis for your new code.

bidParams1.desrialize(bidParams2.serialize()) == bidParams2 must therefore always be true.

NOTE: Some names are changed between protocol string representation of instance properties:

Contributing to dav-js

Asking for help

We appreciate your effort in taking the time to work on this issue and help out the open source community and the foundation. If you need any help, feel free to ask below or in our gitter channel. We are always happy to help 😄

Aliciawyse commented 5 years ago

Hello! I'd like to do this.

Aliciawyse commented 5 years ago

Any tips on installing jest? I get an error message whether I try yarn add --dev jest or npm install --save-dev jest.

When I run yarn add --dev jest, this is the first error message I see. 

error /Users/aliciabarrett/dav-js/node_modules/sha3: Command failed.
Exit code: 1
Command: node-gyp rebuild
Arguments:
Directory: /Users/aliciabarrett/dav-js/node_modules/sha3
Output:
module.js:549
    throw err;
    ^

Error: Cannot find module '/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js'
    at Function.Module._resolveFilename (module.js:547:15)
    at Function.Module._load (module.js:474:25)
Aliciawyse commented 5 years ago

@TalAter any advice for this kind of error?

TalAter commented 5 years ago

Hi @Aliciawyse, sorry for the slow response time.

I've had a similar issue a while back... I think it had to do with the sha3 library (required by web3) not supporting node version 10.x. Back then I had to downgrade my machine from node.js v10

But I actually just upgraded back to node.js v10 a few minutes ago just to test it, and it actually worked for me now.

Can you try the following and tell me if anything works?

TalAter commented 5 years ago

Also, the project already includes jest, so you don't need to install it. I just upgraded it to the latest version too, so maybe you'd like to pull the latest changes.

Aliciawyse commented 5 years ago

Oh okay, thanks @TalAter! To pull your latest changes, I had to sync my fork with the upstream repository (https://github.com/DAVFoundation/dav-js). Things looks normal now. I tried to push, but couldn't because some tests failed, not sure why but this sorta feels like progress lol.

TalAter commented 5 years ago

We have a bunch of tasks running as a pre-push hook. Linting, testing, spell checking, etc.

What failed?

TalAter commented 5 years ago

Did you get these errors?

1__tal_tals-macbook-pro-5____dropbox_mysite_dav_dav-js__zsh_

I just fixed this and pushed the fix (ebfafdc6f79cd7a699cd309f2e1d3e03f7249573).

Aliciawyse commented 5 years ago

@TalAter Yep that was it. Thanks! Going to give this another try later today!

TalAter commented 5 years ago

I wonder what ended up helping?

Aliciawyse commented 5 years ago

Hmm. Well, less tests are failing. Here's what I'm getting so far.

image

image

image

image

TalAter commented 5 years ago

I googled your error and there are some results around that... but I can't really debug that. You'll have to do it on your machine.

It's not an error I'm familiar with.

I am curious though:

How did you get to 30 commits? 😄 Or is that simply from bringing upstream changes?

Aliciawyse commented 5 years ago

Yeah, I'm thinking that was from upstream changes I grabbed. I'm not sure.

Either way, I got rid of my forked version and forked the project again. I'm able to push without running into the errors like before.

But, I've hit another wall.

How do I run my test?

The jest docs https://jestjs.io/docs/en/cli.html offer some options, but none of them work.

I've tried jest src/vessel-charging/NeedFilterParams.test.ts to see if I can run an existing test. I tried npm test but get a message that saysError: no test specified. I specify a test like npm test src/vessel-charging/NeedFilterParams.test.ts and that gets me the same error message.

TalAter commented 5 years ago

You can run it with npm run jest

Aliciawyse commented 5 years ago

Alright, I'm making progress on this. Still trying to understand how to write a test for the deserialize method. Here's what I'm working with so far... https://github.com/Aliciawyse/dav-js/blob/master/src/vessel-charging/BidParams.test.ts

Aliciawyse commented 5 years ago

Hey @TalAter or @srfrnk

(I figured I'd tag both of you since y'all are the top contributors of this repo).

Any tips on getting the deserialize test to pass? Here's what I've got so far

https://github.com/Aliciawyse/dav-js/blob/master/src/vessel-charging/BidParams.test.ts

I figured I was taking the wrong approach to the deserialize test so I rewrote it like this (below), but it still fails. 

describe('deserialize method', () => {
        it('should return BidParams instance with the original parameters', () => {
            const bidParamsObject = new BidParams();
            bidParamsObject.deserialize(serializedBidParams);
            expect(bidParamsObject).toEqual(bidParams);
        });
    });

Here's a snapshot about why my test is failing. It doesn't look like the deserialize method is changing serializedBidParams. 

- Expected
+ Received
    -   "manufacturer": "manufacturer_name",
    -   "model": "model_name",
    +   "manufacturer": Array [
    +     5,
    +   ],
    +   "model": Array [
    +     5,
    +   ],
        "neederDavId": "davId",
    -   "price": "100000000000000000",
    -   "provider": "N3m0",
    +   "price": Array [
    +     "100000000000000000",
    +   ],
    +   "protocol": "boat_charging",
    +   "provider": Array [
    +     5,
    +   ],
    +   "ttl": undefined,
    +   "type": "bid",

Am I misunderstanding how the deserialize method works?

TalAter commented 5 years ago

This seems to be a bug in vessel charging when setting amenities. It not only sets the amenities but also the manufacturer, model, and provider to the value of the amenities. I opened a bug for this.

In the meantime, check out my pull request to your fork which removes those bad params - https://github.com/Aliciawyse/dav-js/pull/1

Aliciawyse commented 5 years ago

Thank you so much!

TalAter commented 5 years ago

Merged 🎆

Thank you so much for the help @Aliciawyse

Aliciawyse commented 5 years ago

lol no thank you! I appreciate all of the feedback and help @TalAter!