Skip to content

Conversation

@folkengine
Copy link
Contributor

@folkengine folkengine commented Feb 1, 2024

Adds unit tests for lib/ocpp/v16/smart_charging.cpp SmartChargingHandler::validate_profile method as well as tests for the add_charge_point_max_profile, add_tx_default_profile and add_tx_profile methods.

Validate Profile

SmartChargingHandler::validate_profile covers the following positive (returns true) and negative (returns false) boundary conditions:

Positive Boundary Conditions:

  • PB01 Valid Profile
  • PB02 Valid Profile No startSchedule & handler allows no startSchedule & profile.chargingProfileKind == Absolute
  • PB03 Valid Profile No startSchedule & handler allows no startSchedule & profile.chargingProfileKind == Relative
  • PB04 Absolute ChargePointMaxProfile Profile with connector id 0
  • PB05 Absolute TxDefaultProfile
  • PB06 Absolute TxProfile ignore_no_transaction == true
  • PB07 Absolute TxProfile && connector transaction != nullptr && transaction_id matches SKIPPED: was not able to test

Negative Boundary Conditions:

  • NB01 Valid Profile, ConnectorID gt this->connectors.size()
  • NB02 Valid Profile, ConnectorID lt 0
  • NB03 profile.stackLevel lt 0
  • NB04 profile.stackLevel gt profile_max_stack_level
  • NB05 profile.chargingProfileKind == Absolute && !profile.chargingSchedule.startSchedule
  • NB06 Number of installed Profiles is > max_charging_profiles_installed
  • NB07 Invalid ChargingSchedule
  • NB08 profile.chargingProfileKind == Recurring && !profile.recurrencyKind
  • NB09 profile.chargingProfileKind == Recurring && !startSchedule
  • NB10 profile.chargingProfileKind == Recurring && !startSchedule && !allow_charging_profile_without_start_schedule
  • NB11 Absolute ChargePointMaxProfile Profile with connector id not 0
  • NB12 Absolute TxProfile connector_id == 0

Add Profile Tests

  • Add ChargePointMaxProfile
  • Add ChargePointMaxProfile accepts a profile that is not a ChargePointMaxProfile
  • Add TxDefaultProfile with ConnectorID of 0 Retrieved as ConnectorID of 1
  • Add TxDefaultProfile with ConnectorID of 0 demonstrating that no profiles are returned with a ConnectorID of 0
  • Add TxDefaultProfile with ConnectorID > 0
  • Add TxDefaultProfile demonstrating how to trigger exception if the connector_id is greater than the number of connectors in the SmartChargingHandler's connectors map.
  • Add TxProfile

Steps to run:

cmake  -B build -G Ninja -DBUILD_TESTING=O -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX="dist" -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
ninja -j$(nproc) -C build install
ninja -j$(nproc) -C build test 

@folkengine folkengine closed this Feb 1, 2024
@folkengine folkengine reopened this Feb 1, 2024
@folkengine folkengine force-pushed the folkengine/add_1.6_validate_and_add_profile_tests branch from c78340e to 0c7c863 Compare February 1, 2024 17:17
@folkengine
Copy link
Contributor Author

@shankari please review

Copy link

@shankari shankari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor changes, once they are done, I am happy to pass it over to the community

@folkengine folkengine force-pushed the folkengine/add_1.6_validate_and_add_profile_tests branch 2 times, most recently from 44c0c11 to 692a705 Compare February 2, 2024 16:57
@shankari
Copy link

shankari commented Feb 3, 2024

@folkengine this is also failing static code analysis now

@folkengine
Copy link
Contributor Author

@folkengine this is also failing static code analysis now

@shankari Yeah, I've been trying to figure out how to turn off the lint, since it's actually wrong. Those fields are a part of the test fixture. Removing them makes the files not compile. Going to need to figure out how to refactor them without copying them over and over. Alas, their linter doesn't let you disable a lint for a single line.

@folkengine folkengine force-pushed the folkengine/add_1.6_validate_and_add_profile_tests branch from bda9130 to 921899e Compare February 5, 2024 15:04
@folkengine
Copy link
Contributor Author

@shankari unfortunately, the refactoring you requested in moving the GTest test fixture createde Codacy linting issues, saying that class fields weren't being used, even though they were. Could not find a way in their docs to ignore specific lints on certain lines unlike other linters. It seems that the tool has bugs dealing with Google Test.

I have moved the fixture back to being inside the test to get past the issue. We can escalate this issue, but that will slow us down.

@folkengine folkengine force-pushed the folkengine/add_1.6_validate_and_add_profile_tests branch 2 times, most recently from 45a4ca2 to f74628b Compare February 26, 2024 15:34
@folkengine folkengine marked this pull request as ready for review February 26, 2024 17:05
@folkengine
Copy link
Contributor Author

@shankari is this 👍?

@Pietfried
Copy link
Contributor

@folkengine can you please resolve the conflicts that occured up after merging: #447

@shankari
Copy link

@folkengine you need to make sure that all the commits are signed

Copy link

@shankari shankari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am approving pending addressing the DCO

Signed-off-by: Christoph <367712+folkengine@users.noreply.github.com>
@folkengine folkengine force-pushed the folkengine/add_1.6_validate_and_add_profile_tests branch from 8bcbc68 to 7529b37 Compare March 13, 2024 18:17
@folkengine
Copy link
Contributor Author

I am approving pending addressing the DCO

Fixed. Thanks @shankari !

@folkengine
Copy link
Contributor Author

folkengine commented Mar 13, 2024

@folkengine can you please resolve the conflicts that occured up after merging: #447

@Pietfried 'Tis done. Thanks!

@Pietfried Pietfried merged commit b530186 into EVerest:main Mar 15, 2024
@folkengine folkengine deleted the folkengine/add_1.6_validate_and_add_profile_tests branch March 15, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants