From 6dc4eaffa36f9ad6aced2a71c598f7e685a944f9 Mon Sep 17 00:00:00 2001
From: Grigoris Pavlakis <grigpavl@ece.auth.gr>
Date: Sat, 17 Aug 2019 21:53:35 +0300
Subject: [PATCH] Improve tests

---
 src/Services/ParameterService.cpp  |  3 +-
 test/Services/ParameterService.cpp | 72 +++++++++++++++++-------------
 2 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/src/Services/ParameterService.cpp b/src/Services/ParameterService.cpp
index a1933cbc..a89a8ceb 100644
--- a/src/Services/ParameterService.cpp
+++ b/src/Services/ParameterService.cpp
@@ -35,8 +35,7 @@ void ParameterService::reportParameterIds(Message& paramIds) {
 	                            ErrorHandler::AcceptanceErrorType::UnacceptableMessage);
 
 	uint16_t numOfIds = paramIds.readUint16();  // number of parameter IDs carried in the message
-//	uint16_t numOfValidIds = 0; // number of IDs that are actually included in the list
-//	reqParam.skipBytes(2); // skip the first 16 bits where the number of valid IDs will be included
+
 	reqParam.appendUint16(numOfValidIds(paramIds)); // include the number of valid IDs
 
 	for (uint16_t i = 0; i < numOfIds; i++) {
diff --git a/test/Services/ParameterService.cpp b/test/Services/ParameterService.cpp
index 7c6f8caa..d68fc7c9 100644
--- a/test/Services/ParameterService.cpp
+++ b/test/Services/ParameterService.cpp
@@ -16,63 +16,73 @@ void foo(ValueType* bar) {  // sample function
 */
 
 TEST_CASE("Parameter Service - General") {
-	SECTION("Parameter Setup") {
-		pserv.addNewParameter(3, 14);  // this one has ID 0
-		pserv.addNewParameter(1, 7, 12);  // this one has 1
-		pserv.addNewParameter(4, 12, 3, nullptr);  // this one has 2
-		pserv.addNewParameter(12, 3, 6, &foo); // this one has 3
-		pserv.addNewParameter(15, 7, 3, &foo); //and this one 4
-	}
-
 	SECTION("Addition to full map") {
-		CHECK(pserv.addNewParameter(15, 5, 4));
+		pserv.addNewParameter(3, 14);  // this one has ID 1
+		pserv.addNewParameter(1, 7, 12);  // this one has 2
+		pserv.addNewParameter(4, 12, 3, nullptr);  // this one has 3
+		pserv.addNewParameter(12, 3, 6, &foo); // this one has 4
+		pserv.addNewParameter(15, 7, 3, &foo); //and this one 5
+
+		REQUIRE_FALSE(pserv.addNewParameter(15, 5, 4));  // addNewParameter should return false
+		Services.reset();  // reset all services
 	}
+
+//	SECTION("Addition of already existing parameter") {
+//
+//	}
 }
 
 TEST_CASE("Parameter Report Subservice") {
+//	SECTION("Empty parameter report") {
+//
+//	}
 
-	SECTION("Faulty Instruction Handling Test") {
-		Message request(20, 1, Message::TC, 1);
-		Message report(20, 2, Message::TM, 1);
+	SECTION("Faulty instruction handling") {
+		pserv.addNewParameter(3, 14);  // ID 1
+		pserv.addNewParameter(1, 7, 12);  // ID 2
+		pserv.addNewParameter(4, 12, 3, nullptr);  // ID 3
 
+		Message request(20, 1, Message::TC, 1);
 		request.appendUint16(2); // number of requested IDs
-		request.appendUint16(65535); // faulty ID in this context
+		request.appendUint16(65535); // invalid ID in this context
 		request.appendUint16(1); // valid
 
 		MessageParser::execute(request);
+		Message report = ServiceTests::get(1);
 
-		CHECK(((ServiceTests::get(0).messageType == 4) && (ServiceTests::get(0).serviceType == 1)));
+		CHECK(ServiceTests::get(0).messageType == 4);
+		CHECK(ServiceTests::get(0).serviceType == 1);
 		// check for an ST[1,4] message caused by the faulty ID
 		CHECK((ServiceTests::thrownError(ErrorHandler::ExecutionStartErrorType::UnknownExecutionStartError)));
 		// check for the thrown UnknownExecutionStartError
-		CHECK(((ServiceTests::get(1).messageType == 2) && (ServiceTests::get(1).serviceType == 20)));
+
+		CHECK(report.messageType == 2);
+		CHECK(report.serviceType == 20);
 		// check for an ST[20,2] message (the one that contains the settings)
 
-		ServiceTests::reset();
-	}
+		CHECK(report.readUint16() == 1);  // only one parameter shall be contained
 
+		CHECK(report.readUint16() == 1);  // check for parameter ID
+		CHECK(report.readUint32() == 12); // check for value (defined when adding parameters)
+
+		ServiceTests::reset();  // clear all errors
+		Services.reset();  // reset the services
+	}
 
-	// **WARNING**
-	// TODO: Update this test (and all tests in general) to use the error handler's output when
-	//  checking for assertions.
 	SECTION("Wrong Message Type Handling Test") {
-		Message falseRequest(15, 3, Message::TM, 1); // a totally wrong message
+		Message falseRequest(62, 3, Message::TM, 1); // a totally wrong message
 
 		MessageParser::execute(falseRequest);
-		Message errorNotif = ServiceTests::get(0);
-		CHECK(errorNotif.messageType == 4); // check for proper failed start of
-		// execution notification
-		CHECK(errorNotif.serviceType == 1);
-
-		Message response = ServiceTests::get(1);
-		CHECK(response.messageType == 2);
-		CHECK(response.serviceType == 20);
-		CHECK(response.packetType == Message::TM);
-		CHECK(response.readPosition == 0); // if empty, this should't change from 0
+		CHECK(ServiceTests::thrownError(ErrorHandler::InternalErrorType::OtherMessageType));
+
+		ServiceTests::reset();
+		Services.reset();
 	}
 }
 
 TEST_CASE("Parameter Setting Subservice") {
+	//SECTION("Passing of null-pointer as update function on construction")
+
 	SECTION("Faulty Instruction Handling Test") {
 		Message setRequest(20, 3, Message::TC, 1);
 		Message reportRequest(20, 1, Message::TC, 1);
-- 
GitLab