fix(moon): changed to type specific return values and added moon test

[godind]

Fixes #122

Previous implementation retuned all moon path values as strings. Adjusted return types to correct types and added tests.

[tkurki]

The moon values are numbers when coming from suncalc at https://github.com/SignalK/signalk-derived-data/blob/master/calcs/moon.js#L25. In this PR this plugin converts them first to strings and then back to numbers, in different parts of the code, that makes very little sense.

[godind]

You are most probably right! To be honest I was just trying to fix the output to get cleaner types and did not look deeper. But I added some tests! It's just a quick patch.

Do you need more work done or is it good for now?

[tkurki]

Could you please fix the issues I pointed out.

[godind]

No problem. I'll look at it.

[tkurki]

There are 2 failing tests on my machine:

       Sets environment.moon.* information such as phase, rise, and set[0] works:

      AssertionError: expected [ { …(2) }, { …(2) }, { …(2) }, …(5) ] to have the same json representation as [ { …(2) }, { …(2) }, { …(2) }, …(5) ]
      + expected - actual

           "value": 1.6
         }
         {
           "path": "environment.moon.times.rise"
      -    "value": "2017-04-15T02:59:40.750Z"
      +    "value": "2017-04-16T03:52:46.109Z"
         }
         {
           "path": "environment.moon.times.set"
      -    "value": "2017-04-15T13:26:01.692Z"
      +    "value": "2017-04-15T13:25:54.314Z"
         }
         {
           "path": "environment.moon.times.alwaysUp"
           "value": false

[godind]

I'll check this out. I dont understand why the date changes every time in tests since it provides static input coordinates and date time.

Could it be that while running the tests, if the server is playing stream file, that it overrides the static test data?

[godind]

I've fix the test issue, removed unnecessary variables, set default as undefined instead of ''. See if this is acceptable.

Thanks!

[tkurki]

There is no server involved in running the tests. I have no idea why the value would change, but the same tests still fail on my machine. Just remove the offending ones?

[godind]

It should work fine. Looks good on my end.

derived data converts
    ✓ Magnetic Course Over Ground[0] works
    ✓ Magnetic Variation[0] works
    ✓ Magnetic Variation[1] works
    ✓ Sets environment.moon.* information such as phase, rise, and set[0] works
    ✓ propulsion.port.slip (propulsion.port.transmission.gearRatio, propulsion.port.drive.propeller.pitch)[0] works
    ✓ propulsion.port.slip (propulsion.port.transmission.gearRatio, propulsion.port.drive.propeller.pitch)[1] works
  6 passing (10ms)

[tkurki]

It seems that the output varies for some reason, the tests fail on my machine

         {
           "path": "environment.moon.times.rise"
      -    "value": "2017-04-15T02:59:40.750Z"
      +    "value": "2017-04-16T03:52:46.109Z"
         }
         {
           "path": "environment.moon.times.set"
      -    "value": "2017-04-15T13:26:01.692Z"
      +    "value": "2017-04-15T13:25:54.314Z"
         }

Why don't you just remove the offending tests where the output varies for some reason? This PR does not change how the value is calculated, so this is a sidetrack.

Unless you want to really dig into what happens and fix things for all.

As long as the test fails I won't be merging this.

[godind]

Done!

[tkurki]

The test is failing, because the output contains properties not in the expected block. You would have spotted that if you had ran the tests.

[godind]

I did not. I have a new machine and did not have complete setup so I just deleted the tests. That's what being out of time does... I'll get to it.

[tkurki]

Let's get this finished?

[godind]

Removed ofending tests.

[godind]

Removed offending tests.

[godind]

Do I still need to fix something? I don't see it if so...

Thanks