Skip to content

enhance meter test code #1439

Description

@huss

Is your feature request related to a problem? Please describe.

src/server/test/web/meters.js has the comment:

// TODO These tests are not as good as they should be now that information on
// meters is returned to all users. They should be updated.

src/server/test/db/meterTests.js has the comment:

//  TODO add 2 new unit parameters for meters.

src/server/test/web/csvPipelineTest.js has the comment:

// TODO It would be nice to make this use the code in src/server/test/db/meterTests.js and make
// all meter tests use one common function fo meter comparison.

These are somewhat correct but more work is needed.

Describe the solution you'd like

This following tests should be added to src/server/test/web/meters.js:

  • src/server/models/Meter.js in the constructor shows there are a number of fields/values that are not checked. They need to be added to expectMetersToBeEquivalent and as values in the tests that specify meter data.
  • For a few values, Meter.convertUnitValue is called to convert the value between null and -99 for JS vs database value. A test should be created to verify that the -99 value is correctly returned to know the conversion works both ways.
  • insert and update use Meter.convertUnitValue to make sure the values are correct. Tests should be created where the changed values are different from the desired values and the returned values are the correct.
  • The tests in meterTests.js should be rolled into meters.js and updated for new value/test as noted in items above this one. meters.js tests the route which then uses the model and database. OED has been doing tests via the route since any failure on the server side is found. The negative is you don't know where on the server the failure occurs but since they are rare this seems fine given it simplifies the test code and the source can be located once a test fails. Once complete, meterTests.js can be removed so its TODO is indirectly resolved.
  • The test code checks insert and getting a single meter id. It would be good if it checked update and the other get functions that were not added when meterTests.js was rolled into this file.
  • src/server/test/web/csvPipelineTest.js should be modified to remove compareMeters and import expectMetersToBeEquivalent from src/server/test/web/meters.js and use that instead. This addresses the TODO in this file but does it by using a different, common test function.

Once all are complete the TODO comments can be removed and the TODO design document updated to reflect it is complete.

Describe alternatives you've considered

None

Additional context

None

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

i-good-first-issueThis issue is probably a good starting point for people new to coding or the OED project.p-low-priorityreservedExpected to be assigned to developer by project so others should not work on without prior approval.t-enhancementThis issues tracks a potential improvement to the software

Type

No type

Fields

Priority

None yet

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions