add tests to check missionsCompleted7Days value is in required range

[cg-cnu]

first-timers-only

This issue is tagged 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 here:

• Missions
• Mission Control
• Captain

Thank you for your help ❤️

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.

Mission Control

The DAV project you are looking at is Mission Control. It is the brain in charge of orchestrating missions between DAV users and autonomous vehicles.

How you can help

Background

To help developers building on top of DAV technologies, Mission Control can start in a simulation environment. In a simulation environment, there are always a few simulated drones flying around the user, ready to take on missions. This makes it easy for developers to start building and testing without investing in hardware.

The Issue

As a project that relies on a large community of contributors, it is very important for us to have good tests to make sure changes don't break anything.

One of the functions that need testing is randomMissionsCompleted() inside /server/simulation/random.js that creates random number of missionsCompleted and missionsCompleted7Days.

The task for this issue is to test the functions return value.

• missionsCompleted7Days should be a positive integer that is less than or equal to the value of missionsCompleted

Edit test/specs/simulation.random.spec.js, adding a describe block for randomMissionsCompleted() if it does not exists and within it one test that verify the condition mentioned above.

To run your tests, run npm test from the project's root directory. All tests should pass.

Contributing to Mission Control

• Fork the repository from the Mission Control GitHub page.
• Clone a copy to your local machine with $ git clone git@github.com:YOUR-GITHUB-USER-NAME/missioncontrol.git
• Make sure you have node.js and npm installed on your machine. You can use this guide for help.
• Install all of the project's dependencies with npm. $ cd missioncontrol; npm install
• Run npm test to run linting checks and all the automated tests and see that they pass.
• Code! code! code!
• Before committing your code, run npm test one last time and make sure no errors (including linting errors) are thrown.
• Once you've made sure all your changes work correctly and committed all your changes, push your local changes back to github with $ git push -u origin master
• Visit your fork on GitHub.com (https://github.com/YOUR-USER-NAME/missioncontrol) and create a pull request for your changes.
• Make sure your pull request describes exactly what you changed and references this issue (include the issue number in the title like this: #30)

[TalAter]

This issue is still available for first-time contributors

The commits listed above and just below this message are for a different issue

[sblasa]

@TalAter @cg-cnu Hi, I would like to try help with this one if that's okay.

[TalAter]

Go for it @sblasa!

Thank you 🙇

[sblasa]

@TalAter Thanks! 😄

[TalAter]

Hey @sblasa, how is it going with this one? Do you need any help with it?

[sblasa]

@TalAter Hi! Sorry, got busy with work. Starting on it today. Thanks for the reminder! :)

[sblasa]

@TalAter I'm getting this error when I run the npm test. I followed directions noted above description.

C:\Users\stbla\code_school\opensource\missioncontrol [master ≡ +0 ~2 -0 !]> npm test

missioncontrol@0.0.1 test C:\Users\stbla\code_school\opensource\missioncontrol
gulp jest

[08:12:49] Using gulpfile ~\code_school\opensource\missioncontrol\gulpfile.js
[08:12:49] Starting 'jest'...
C:\Users\stbla\code_school\opensource\missioncontrol\node_modules\jest-haste-map\build\crawlers\node.js:36
names.forEach(file => {
^

TypeError: Cannot read property 'forEach' of undefined
at fs.readdir (C:\Users\stbla\code_school\opensource\missioncontrol\node_modules\jest-haste-map\build\crawlers\node.js:36:12)
at go$readdir$cb (C:\Users\stbla\code_school\opensource\missioncontrol\node_modules\jest-cli\node_modules\graceful-fs\graceful-fs.js:149:14)
at FSReqWrap.oncomplete (fs.js:135:15)
npm ERR! Test failed. See above for more details.

[TalAter]

Hmmm. I cannot reproduce this on my machine and there is nothing specific about Mission Control code in the error.

My guess would be it has something to do with the script trying to access the filesystem in an environment that isn't unix-like.

Are you using the standard/old school command shell of Windows? If you are using Windows 10, I would try using the new bash-like shell they offer. If you are in an older version I know I used to use cygwin back in my Windows days, but there may be better solutions these days.

I know this may seem like a lot of hoops to jump for just to send a PR for this project, but I think it is worth it because this has far reaching consequences for most open source projects you'll work with.

[sblasa]

@TalAter Hi, yes, I tried using Git Bash, and that didn't work as well. I'm going to redo the repo on my machine and run it again. I will keep trying! :) Thanks!

[sblasa]

@TalAter Hi, I figured out that I needed to do a pull from the original repo and merge it with my project. I had noticed that there were some file changes in the original missioncontrol that I didn't have. It's working now. Moving on to fix the issue at hand. :)

[sblasa]

@TalAter , so I'm looking at the est/specs/simulation.random.spec.js . There is a describe block in there already, but I needed some help. I added the test in bold below and it passed. However, I need to add a test to see if the missionsCompleted7Days is a positive integer. Could you tell me how I could add that to the test?

Thanks!

describe('randomMissionsCompleted()', () => {

test('returns an object', () => {
expect(
typeof randomMissionsCompleted()
).toBe('object');
});

test('returns an object containing missionsCompleted7Days which is less than or equal to missionsCompleted', () => {
expect(
randomMissionsCompleted().missionsCompleted7Days
).toBeLessThanOrEqual(randomMissionsCompleted().missionsCompleted);
});

test('returns an object containing missionsCompleted which is an integer between 4 and 90', () => {
expect(
Number.isInteger(randomMissionsCompleted().missionsCompleted)
).toBeTruthy();
expect(
randomMissionsCompleted().missionsCompleted
).toBeLessThanOrEqual(90);
expect(
randomMissionsCompleted().missionsCompleted
).toBeGreaterThanOrEqual(4);
});

});

[Rob-Rychs]

Hey @sblasa,
Maybe try something like .toBeGreaterThan(0) ?
That will at least check to make sure it's a positive number, if not a whole number which I'm not sure is necessary to test for
https://facebook.github.io/jest/docs/en/expect.html#tobegreaterthannumber
Just my thoughts...
😃

[TalAter]

Thanks @Rob-Rychs

@sblasa does that answer your question?

GitHub Pro Tip #3704: When entering code in these GitHub issues, it is best to use markdown to make it easier to read... You even get syntax highlighting for free with it:

Just surround your code with some backticks like so:

describe('randomMissionsCompleted()', () => {

  test('returns an object', () => {
    expect(
      typeof randomMissionsCompleted()
    ).toBe('object');
  });

});

[sblasa]

@TalAter Ah, thanks for the tip! I was wondering how I would do that. 👍 I'm going to try out the code that Rob-Rychs mentioned.
@Rob-Rychs thank you for your help! I will try that and see if it works. 👍

[sblasa]

@TalAter The code passed when I ran the test. I added this code

test('returns an object containing missionsCompleted7Days which is less than or equal to missionsCompleted', () => {
      expect(
        randomMissionsCompleted().missionsCompleted7Days
      ).toBeLessThanOrEqual(randomMissionsCompleted().missionsCompleted);
  
  });

  test('returns an object containing missionsCompleted7Days which is an integer', () => {      
      expect(
        Number.isInteger(randomMissionsCompleted().missionsCompleted7Days)
      ).toBeTruthy();     
  
  });

  test('returns an object containing missionsCompleted7Days which is a positive number', () => {     
      expect(
        randomMissionsCompleted().missionsCompleted7Days
      ).toBeGreaterThan(0);

  });

What do you think? Thanks for your help!

[TalAter]

• The following is just a stylistic choice, but I think we should stick to the same format of test someone has already written for missionsCompleted (https://github.com/DAVFoundation/missioncontrol/blob/master/test/specs/simulation.random.spec.js#L43), which is to include all of them in one test that asserts that "missionsCompleted7Days should be a positive integer that is less than or equal to the value of missionsCompleted".

• I would replace the condition toBeTruthy() with a more strict toBe(true). toBeTruthy would pass on anything that is truthy (e.g. true, 17.3, "Esmarelda Villalobos"). This shouldn't be a problem here, since Number.isInteger always returns a boolean... but why not be more strict when we can? 😃

• Your first test might fail from time to time because you are calling randomMissionsCompleted() twice. We want to verify that each time we call it, it will contain a missionsCompleted7Days that is smaller than the missionsCompleted it returned this time. But since you call it twice, you might get {missionsCompleted7Days: 2, missionsCompleted: 4} the first time you call it, and {missionsCompleted7Days: 10, missionsCompleted: 100} the second time which would fail your test (10 < 4) even though it satisfied the requirement we want to test (10 < 100)

[Rob-Rychs]

Good stuff @sblasa @TalAter
If we store the original call to missionsCompleted() and then run the test using the properties from the stored call that might help prevent the issue with it failing from time to time...
something like...

test('returns an object containing missionsCompleted7Days which is less than or equal to missionsCompleted', () => {
   const prop = randomMissionsCompleted();
   expect(
     prop.missionsCompleted7Days
   ).toBeLessThanOrEqual(prop.missionsCompleted);

 }); `

[sblasa]

@TalAter thanks! Those are good points. I will change the format of that and change the conditions. @Rob-Rychs, that sounds like a good idea. I will try that too. Thank you you both for being very supportive. :) I will implement this tonight.

[TalAter]

@sblasa Are you still interested in working on this issue? We have received another pull request for it by @JordanShaak

[sblasa]

@TalAter @Rob-Rychs Yes, I finished the update yesterday, and I was just going back through steps on Pull Request. I am submitting the pull request. Thanks! :)

[TalAter]

🎊 Just merged it. Awesome!

Congratulations on being an open sourcerer, and thank you so much for taking the time to help build the project!

[sblasa]

@TalAter @Rob-Rychs Thanks for all your help! I'll keep an eye out for any other issues that I can help with. :)