From 9ba31f6d09c8ce2437d32124184c8f1af323a534 Mon Sep 17 00:00:00 2001 From: Neelam Kushwah Date: Sat, 6 Apr 2024 11:06:36 -0400 Subject: [PATCH 1/6] Add workflow to test code coverage and generate coverage report This commit adds a GitHub Actions workflow (`coverage.yml`) . The workflow runs on pull requests targeting the `main` branch and consists of a job named `test-and-coverage`. The job runs tests with coverage enabled, generates an HTML coverage report, and uploads the report as an artifact. Additionally, the workflow extracts the pull request title and description and comments on the pull request with the coverage results. --- .github/workflows/coverage.yml | 86 ++++++++++++++++++++++++++++++++++ .gitignore | 3 +- README.adoc | 2 +- pyproject.toml | 4 ++ 4 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/coverage.yml diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml new file mode 100644 index 0000000..fda24a0 --- /dev/null +++ b/.github/workflows/coverage.yml @@ -0,0 +1,86 @@ +name: Python Test and Coverage + +on: + pull_request: + branches: + - main + + +jobs: + test-and-coverage: + name: Test with coverage + runs-on: ubuntu-latest +# permissions: +# contents: write +# pull-requests: write +# repository-projects: write +# id-token: write + + steps: + - name: Checkout code + uses: actions/checkout@v3 + + - name: Set up Apache Maven Central + uses: actions/setup-java@v3 + with: # configure settings.xml + distribution: 'temurin' + java-version: '11' + server-id: ossrh + server-username: OSSRH_USER + server-password: OSSRH_PASSWORD + + - name: Set up Python + uses: actions/setup-python@v3 + with: + python-version: '3.x' + + - name: Install Poetry + run: | + python -m pip install --upgrade pip + python -m pip install poetry + + - name: Install dependencies + run: | + poetry install + + - name: Run prebuild script + run: | + cd scripts + # Run the script within the Poetry virtual environment + poetry run python pull_and_compile_protos.py + + - name: Run tests with coverage + run: | + poetry run coverage run --source=uprotocol --omit=uprotocol/proto/*,uprotocol/cloudevent/*_pb2.py,tests/*,*/__init__.py -m pytest + poetry run coverage report > coverage_report.txt + export COVERAGE_PERCENTAGE=$(awk '/TOTAL/{print $4}' coverage_report.txt) + echo "COVERAGE_PERCENTAGE=$COVERAGE_PERCENTAGE" >> $GITHUB_ENV + echo "COVERAGE_PERCENTAGE: $COVERAGE_PERCENTAGE" + poetry run coverage html + + + - name: Upload coverage report + uses: actions/upload-artifact@v2 + with: + name: coverage-report + path: htmlcov/ + +# - name: Comment PR with coverage results +# uses: actions/github-script@v6 +# with: +# script: | +# const COVERAGE_PERCENTAGE = process.env.COVERAGE_PERCENTAGE;; +# const COVERAGE_REPORT_PATH = `https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}/`; +# +# github.rest.issues.createComment({ +# issue_number: context.issue.number, +# owner: context.repo.owner, +# repo: context.repo.repo, +# body: ` +# Code coverage report is ready! :chart_with_upwards_trend: +# +# - **Code Coverage Percentage:** ${COVERAGE_PERCENTAGE} +# - **Code Coverage Report:** [View Coverage Report](${COVERAGE_REPORT_PATH}) +# ` +# }); + diff --git a/.gitignore b/.gitignore index f130fb9..75de8c0 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,5 @@ .coverage target #**/proto -poetry.lock \ No newline at end of file +poetry.lock +htmlcov \ No newline at end of file diff --git a/README.adoc b/README.adoc index 48d2967..aee2c81 100644 --- a/README.adoc +++ b/README.adoc @@ -112,4 +112,4 @@ Clean up by running the command: Requires coverage to be installed first, that can be done by running `pip install coverage` then you run: -`python -m coverage run --source tests/ -m unittest discover` +`python -m coverage run --source tests/ -m unittest discover` \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml index 275cdc9..1c22c14 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,6 +20,10 @@ gitpython = ">=3.1.41" googleapis-common-protos = ">=1.56.4" protobuf = "4.24.2" +[tool.poetry.dev-dependencies] +pytest = "^6.2" +coverage = "^5.5" + [build-system] requires = ["poetry-core"] build-backend = "poetry.core.masonry.api" \ No newline at end of file From 6eb4333ddf1478873dc97aeff47ba63d4bc1c55b Mon Sep 17 00:00:00 2001 From: Neelam Kushwah Date: Sat, 6 Apr 2024 20:19:18 -0400 Subject: [PATCH 2/6] Add utransport test cases --- tests/test_transport/test_utransport.py | 96 +++++++++++++++++++ .../datamodel/ucloudeventattributes.py | 13 --- 2 files changed, 96 insertions(+), 13 deletions(-) create mode 100644 tests/test_transport/test_utransport.py diff --git a/tests/test_transport/test_utransport.py b/tests/test_transport/test_utransport.py new file mode 100644 index 0000000..bd45b6e --- /dev/null +++ b/tests/test_transport/test_utransport.py @@ -0,0 +1,96 @@ +from uprotocol.transport.ulistener import UListener +from uprotocol.transport.utransport import UTransport +from uprotocol.proto.ustatus_pb2 import UStatus, UCode +from uprotocol.proto.umessage_pb2 import UMessage +import unittest +from uprotocol.proto.uri_pb2 import UUri + + +class MyListener(UListener): + def on_receive(self, message): + super().on_receive( message) + pass + + +class HappyUTransport(UTransport): + + def send(self, message): + super().send( message) + + return UStatus( + code=UCode.INVALID_ARGUMENT if message is None else UCode.OK) + + def register_listener(self, topic, listener): + super().register_listener( topic, listener) + listener.on_receive(UMessage()) + return UStatus(code=UCode.OK) + + def unregister_listener(self, topic, listener): + super().unregister_listener(topic, listener) + return UStatus(code=UCode.OK) + + +class SadUTransport(UTransport): + def send(self, message): + super().send( message) + return UStatus(code=UCode.INTERNAL) + + def register_listener(self, topic, listener): + super().register_listener( topic, listener) + listener.on_receive(None) + return UStatus(code=UCode.INTERNAL) + + def unregister_listener(self, topic, listener): + super().unregister_listener( topic, listener) + return UStatus(code=UCode.INTERNAL) + + +class UTransportTest(unittest.TestCase): + def test_happy_send_message_parts(self): + transport = HappyUTransport() + status = transport.send(UMessage()) + self.assertEqual(status.code, UCode.OK) + + def test_happy_send_message(self): + transport = HappyUTransport() + status = transport.send(UMessage()) + self.assertEqual(status.code, UCode.OK) + + def test_happy_register_listener(self): + transport = HappyUTransport() + status = transport.register_listener(UUri(), MyListener()) + self.assertEqual(status.code, UCode.OK) + + def test_happy_register_unlistener(self): + transport = HappyUTransport() + status = transport.unregister_listener(UUri(), MyListener()) + self.assertEqual(status.code, UCode.OK) + + def test_sending_null_message(self): + transport = HappyUTransport() + status = transport.send(None) + self.assertEqual(status.code, UCode.INVALID_ARGUMENT) + + def test_unhappy_send_message_parts(self): + transport = SadUTransport() + status = transport.send(UMessage()) + self.assertEqual(status.code, UCode.INTERNAL) + + def test_unhappy_send_message(self): + transport = SadUTransport() + status = transport.send(UMessage()) + self.assertEqual(status.code, UCode.INTERNAL) + + def test_unhappy_register_listener(self): + transport = SadUTransport() + status = transport.register_listener(UUri(), MyListener()) + self.assertEqual(status.code, UCode.INTERNAL) + + def test_unhappy_register_unlistener(self): + transport = SadUTransport() + status = transport.unregister_listener(UUri(), MyListener()) + self.assertEqual(status.code, UCode.INTERNAL) + + +if __name__ == '__main__': + unittest.main() \ No newline at end of file diff --git a/uprotocol/cloudevent/datamodel/ucloudeventattributes.py b/uprotocol/cloudevent/datamodel/ucloudeventattributes.py index 493542c..c6fc272 100644 --- a/uprotocol/cloudevent/datamodel/ucloudeventattributes.py +++ b/uprotocol/cloudevent/datamodel/ucloudeventattributes.py @@ -213,16 +213,3 @@ def build(self): self.priority, self.hash, self.ttl, self.token, self.traceparent ) - -if __name__ == "__main__": - # Example usage: - attributes = ( - UCloudEventAttributesBuilder() - .with_hash("abc123") - .with_priority(UPriority.UPRIORITY_CS0) - .with_ttl(1000) - .with_token("xyz456") - .with_traceparent("123456") - .build() - ) - print(attributes) From 0028c9846bde457182622f8ddd2f947113fd6ad9 Mon Sep 17 00:00:00 2001 From: Matthew D'Alonzo Date: Tue, 9 Apr 2024 11:44:25 -0400 Subject: [PATCH 3/6] Increase code coverage With these new tests, we were able to increase code coverage to 98%. Along with this, we provided some linting improvements, as well as fixed a few methods that were implemented incorrectly in a previous PR. --- .gitignore | 3 +- pyproject.toml | 8 +- .../test_datamodel/test_ucloudevent.py | 153 ++++++++++++++++++ .../test_ucloudeventattributes.py | 23 +++ .../test_cloudeventvalidator.py | 2 +- .../test_builder/test_uattributesbuilder.py | 88 +++++++++- .../test_builder/test_upayloadbuilder.py | 18 +++ .../test_uattributesvalidator.py | 73 ++++++++- .../test_microuriserializer.py | 6 + .../test_shorturiserializer.py | 9 ++ uprotocol/cloudevent/factory/ucloudevent.py | 14 +- .../transport/builder/uattributesbuilder.py | 31 ++-- .../validate/uattributesvalidator.py | 9 +- 13 files changed, 403 insertions(+), 34 deletions(-) diff --git a/.gitignore b/.gitignore index 75de8c0..63a904c 100644 --- a/.gitignore +++ b/.gitignore @@ -9,4 +9,5 @@ target #**/proto poetry.lock -htmlcov \ No newline at end of file +htmlcov +coverage_report.txt diff --git a/pyproject.toml b/pyproject.toml index 1c22c14..b8e76f4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,4 +26,10 @@ coverage = "^5.5" [build-system] requires = ["poetry-core"] -build-backend = "poetry.core.masonry.api" \ No newline at end of file +build-backend = "poetry.core.masonry.api" + +[tool.coverage.report] +exclude_also = [ + "pass", + "raise NotImplementedError" + ] \ No newline at end of file diff --git a/tests/test_cloudevent/test_datamodel/test_ucloudevent.py b/tests/test_cloudevent/test_datamodel/test_ucloudevent.py index e6f9696..5aae1b2 100644 --- a/tests/test_cloudevent/test_datamodel/test_ucloudevent.py +++ b/tests/test_cloudevent/test_datamodel/test_ucloudevent.py @@ -24,10 +24,13 @@ # # ------------------------------------------------------------------------- +from datetime import datetime, timezone, timedelta + from uprotocol.proto.uri_pb2 import UUri, UEntity, UResource from uprotocol.uri.serializer.longuriserializer import LongUriSerializer from uprotocol.cloudevent.cloudevents_pb2 import CloudEvent from uprotocol.proto.uattributes_pb2 import UMessageType, UPriority +from uprotocol.proto.upayload_pb2 import UPayloadFormat from uprotocol.cloudevent.factory.cloudeventfactory import CloudEventFactory from uprotocol.proto.ustatus_pb2 import UCode from uprotocol.uuid.factory.uuidfactory import Factories @@ -90,6 +93,31 @@ def build_cloud_event_for_test(): return cloud_event +def build_cloud_event_for_test_with_id(id): + source = build_uri_for_test() + proto_payload = build_proto_payload_for_test() + # additional attributes + u_cloud_event_attributes = ( + UCloudEventAttributesBuilder() + .with_hash("somehash") + .with_priority(UPriority.UPRIORITY_CS1) + .with_ttl(3) + .with_token("someOAuthToken") + .build() + ) + + # build the cloud event + cloud_event = CloudEventFactory.build_base_cloud_event( + id, + source, + proto_payload.SerializeToString(), + proto_payload.type_url, + u_cloud_event_attributes, + UCloudEvent.get_event_type(UMessageType.UMESSAGE_TYPE_PUBLISH), + ) + return cloud_event + + class TestUCloudEvent(unittest.TestCase): DATA_CONTENT_TYPE = "application/x-protobuf" @@ -102,6 +130,7 @@ def test_extract_sink_from_cloudevent_when_sink_exists(self): sink = "//bo.cloud/petapp/1/rpc.response" cloud_event = build_cloud_event_for_test() cloud_event.__setitem__("sink", sink) + cloud_event.__setitem__("plevel", 4) self.assertEqual(sink, UCloudEvent.get_sink(cloud_event)) def test_extract_sink_from_cloudevent_when_sink_does_not_exist(self): @@ -193,6 +222,82 @@ def test_extract_platform_error_from_cloudevent_when_platform_error_exists_in_wr UCode.OK, UCloudEvent.get_communication_status(cloud_event) ) + def test_extract_platform_error_from_cloudevent_when_platform_error_exists( + self, + ): + cloud_event = build_cloud_event_for_test() + cloud_event.__setitem__("commstatus", UCode.INVALID_ARGUMENT) + self.assertEqual( + UCode.INVALID_ARGUMENT, + UCloudEvent.get_communication_status(cloud_event), + ) + + def test_extract_platform_error_from_cloudevent_when_platform_error_does_not_exist( + self, + ): + cloud_event = build_cloud_event_for_test() + self.assertEqual( + UCode.OK, UCloudEvent.get_communication_status(cloud_event) + ) + + def test_adding_platform_error_to_existing_cloudevent( + self, + ): + cloud_event = build_cloud_event_for_test() + self.assertEqual( + UCode.OK, UCloudEvent.get_communication_status(cloud_event) + ) + + cloud_event_1 = UCloudEvent.add_communication_status( + cloud_event, UCode.DEADLINE_EXCEEDED + ) + + self.assertEqual( + UCode.OK, UCloudEvent.get_communication_status(cloud_event) + ) + + self.assertEqual( + UCode.DEADLINE_EXCEEDED, + UCloudEvent.get_communication_status(cloud_event_1), + ) + + def test_adding_empty_platform_error_to_existing_cloudevent( + self, + ): + cloud_event = build_cloud_event_for_test() + self.assertEqual( + UCode.OK, UCloudEvent.get_communication_status(cloud_event) + ) + + cloud_event_1 = UCloudEvent.add_communication_status(cloud_event, None) + + self.assertEqual( + UCode.OK, UCloudEvent.get_communication_status(cloud_event) + ) + + self.assertEqual(cloud_event, cloud_event_1) + + def test_extract_creation_timestamp_from_cloudevent_UUID_Id_when_not_a_UUIDV8_id( + self, + ): + cloud_event = build_cloud_event_for_test() + self.assertEqual(None, UCloudEvent.get_creation_timestamp(cloud_event)) + + def test_extract_creation_timestamp_from_cloudevent_UUIDV8_Id_when_UUIDV8_id_is_valid( + self, + ): + uuid = Factories.UPROTOCOL.create() + str_uuid = LongUuidSerializer.instance().serialize(uuid) + cloud_event = build_cloud_event_for_test_with_id(str_uuid) + maybe_creation_timestamp = UCloudEvent.get_creation_timestamp( + cloud_event + ) + self.assertIsNotNone(maybe_creation_timestamp) + creation_timestamp = maybe_creation_timestamp / 1000 + + now_timestamp = datetime.now(timezone.utc).timestamp() + self.assertAlmostEqual(creation_timestamp, now_timestamp, delta=1) + def test_cloudevent_is_not_expired_cd_when_no_ttl_configured(self): cloud_event = build_cloud_event_for_test() cloud_event.__delitem__("ttl") @@ -214,6 +319,14 @@ def test_cloudevent_is_not_expired_cd_when_ttl_is_minus_one(self): UCloudEvent.is_expired_by_cloud_event_creation_date(cloud_event) ) + def test_cloudevent_is_expired_cd_when_ttl_is_one(self): + cloud_event = build_cloud_event_for_test() + cloud_event.__setitem__("ttl", 1) + time.sleep(0.002) + self.assertTrue( + UCloudEvent.is_expired_by_cloud_event_creation_date(cloud_event) + ) + def test_cloudevent_is_expired_when_ttl_1_mili(self): uuid = Factories.UPROTOCOL.create() str_uuid = LongUuidSerializer.instance().serialize(uuid) @@ -294,6 +407,8 @@ def test_from_message_with_valid_message(self): UCloudEvent.get_type(cloud_event1), ) + + def test_to_from_message_from_request_cloudevent(self): # additional attributes u_cloud_event_attributes = ( @@ -444,6 +559,8 @@ def test_to_from_message_from_UCP_cloudevent(self): u_cloud_event_attributes, ) cloud_event.__setitem__("priority", "CS4") + cloud_event.__setitem__("commstatus", 16) + cloud_event.__setitem__("permission_level", 4) result = UCloudEvent.toMessage(cloud_event) self.assertIsNotNone(result) @@ -458,3 +575,39 @@ def test_from_message_with_null_message(self): with self.assertRaises(ValueError) as context: UCloudEvent.fromMessage(None) self.assertTrue("message cannot be null." in context.exception) + + def test_cloud_event_to_string(self): + u_cloud_event_attributes = ( + UCloudEventAttributesBuilder() + .with_ttl(3) + .with_token("someOAuthToken") + .build() + ) + + cloud_event = CloudEventFactory.request( + build_uri_for_test(), + "//bo.cloud/petapp/1/rpc.response", + CloudEventFactory.generate_cloud_event_id(), + build_proto_payload_for_test(), + u_cloud_event_attributes, + ) + cloud_event_string = UCloudEvent.to_string(cloud_event) + self.assertTrue( + "source='/body.access//door.front_left#Door', sink='//bo.cloud/petapp/1/rpc.response', type='req.v1'}" + in cloud_event_string + ) + + def test_cloud_event_to_string_none(self): + cloud_event_string = UCloudEvent.to_string(None) + self.assertEqual( + cloud_event_string, "null" + ) + + def test_get_upayload_format_from_content_type(self): + new_format = UCloudEvent().get_upayload_format_from_content_type("application/json") + self.assertEqual(new_format, UPayloadFormat.UPAYLOAD_FORMAT_JSON) + + def test_to_message_none_entry(self): + with self.assertRaises(ValueError) as context: + UCloudEvent().toMessage(None) + self.assertTrue("Cloud Event can't be None" in context.exception) \ No newline at end of file diff --git a/tests/test_cloudevent/test_datamodel/test_ucloudeventattributes.py b/tests/test_cloudevent/test_datamodel/test_ucloudeventattributes.py index d3d9355..39df384 100644 --- a/tests/test_cloudevent/test_datamodel/test_ucloudeventattributes.py +++ b/tests/test_cloudevent/test_datamodel/test_ucloudeventattributes.py @@ -98,6 +98,29 @@ def test_is_empty_function_permutations(self): u_cloud_event_attributes5 = UCloudEventAttributesBuilder().with_ttl(8).build() self.assertFalse(u_cloud_event_attributes5.is_empty()) + def test__eq__is_same(self): + u_cloud_event_attributes = UCloudEventAttributesBuilder().with_hash(" ").with_token(" ").build() + self.assertTrue(u_cloud_event_attributes.__eq__(u_cloud_event_attributes)) + + def test__eq__is_equal(self): + u_cloud_event_attributes_1 = UCloudEventAttributesBuilder().with_hash(" ").with_token(" ").build() + u_cloud_event_attributes_2 = UCloudEventAttributesBuilder().with_hash(" ").with_token(" ").build() + self.assertTrue(u_cloud_event_attributes_1.__eq__(u_cloud_event_attributes_2)) + + def test__eq__is_not_equal(self): + u_cloud_event_attributes_1 = UCloudEventAttributesBuilder().with_hash(" ").with_token(" ").build() + u_cloud_event_attributes_2 = UCloudEventAttributesBuilder().with_hash(" ").with_token("12345").build() + self.assertFalse(u_cloud_event_attributes_1.__eq__(u_cloud_event_attributes_2)) + + def test__hash__same(self): + u_cloud_event_attributes_1 = UCloudEventAttributesBuilder().with_hash(" ").with_token(" ").build() + self.assertEqual(hash(u_cloud_event_attributes_1), hash(u_cloud_event_attributes_1)) + + def test__hash__different(self): + u_cloud_event_attributes_1 = UCloudEventAttributesBuilder().with_hash(" ").with_token(" ").build() + u_cloud_event_attributes_2 = UCloudEventAttributesBuilder().with_hash(" ").with_token("12345").build() + self.assertNotEqual(hash(u_cloud_event_attributes_1), hash(u_cloud_event_attributes_2)) + if __name__ == '__main__': unittest.main() diff --git a/tests/test_cloudevent/test_validator/test_cloudeventvalidator.py b/tests/test_cloudevent/test_validator/test_cloudeventvalidator.py index 3746d5c..b08a1ff 100644 --- a/tests/test_cloudevent/test_validator/test_cloudeventvalidator.py +++ b/tests/test_cloudevent/test_validator/test_cloudeventvalidator.py @@ -640,4 +640,4 @@ def fetching_the_notification_validator(self): validator = CloudEventValidator.get_validator(cloud_event) status = validator.validate_type(cloud_event).to_status() self.assertEqual(status, ValidationResult.STATUS_SUCCESS) - self.assertEqual("CloudEventValidator.Notification", str(validator)) + self.assertEqual("CloudEventValidator.Notification", str(validator)) \ No newline at end of file diff --git a/tests/test_transport/test_builder/test_uattributesbuilder.py b/tests/test_transport/test_builder/test_uattributesbuilder.py index 9757728..152f683 100644 --- a/tests/test_transport/test_builder/test_uattributesbuilder.py +++ b/tests/test_transport/test_builder/test_uattributesbuilder.py @@ -134,6 +134,7 @@ def test_build(self): .withPermissionLevel(2) .withCommStatus(UCode.CANCELLED) .withReqId(req_id) + .withTraceparent("test_traceparent") ) attributes = builder.build() self.assertIsNotNone(attributes) @@ -146,7 +147,92 @@ def test_build(self): self.assertEqual(2, attributes.permission_level) self.assertEqual(UCode.CANCELLED, attributes.commstatus) self.assertEqual(req_id, attributes.reqid) - + self.assertEqual("test_traceparent", attributes.traceparent) + + def test_publish_source_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.publish(None, UPriority.UPRIORITY_CS1) + self.assertTrue("Source cannot be None." in context.exception) + + def test_publish_priority_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.publish(build_source(), None) + self.assertTrue("UPriority cannot be null." in context.exception) + + def test_notification_source_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.notification( + None, build_sink(), UPriority.UPRIORITY_CS1 + ) + self.assertTrue("Source cannot be None." in context.exception) + + def test_notification_priority_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.notification(build_source(), build_sink(), None) + self.assertTrue("UPriority cannot be null." in context.exception) + + def test_notification_sink_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.notification( + build_source(), None, UPriority.UPRIORITY_CS1 + ) + self.assertTrue("sink cannot be null." in context.exception) + + def test_request_source_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.request( + None, build_sink(), UPriority.UPRIORITY_CS1, 1000 + ) + self.assertTrue("Source cannot be None." in context.exception) + + def test_request_priority_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.request( + build_source(), build_sink(), None, 1000 + ) + self.assertTrue("UPriority cannot be null." in context.exception) + + def test_request_sink_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.request( + build_source(), None, UPriority.UPRIORITY_CS1, 1000 + ) + self.assertTrue("sink cannot be null." in context.exception) + + def test_request_ttl_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.request( + build_source(), build_sink(), UPriority.UPRIORITY_CS1, None + ) + self.assertTrue("ttl cannot be null." in context.exception) + + def test_response_priority_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.response( + build_source(), build_sink(), None, get_uuid() + ) + self.assertTrue("UPriority cannot be null." in context.exception) + + def test_response_sink_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.response( + build_source(), None, UPriority.UPRIORITY_CS1, get_uuid() + ) + self.assertTrue("sink cannot be null." in context.exception) + + def test_response_reqid_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.response( + build_source(), build_sink(), UPriority.UPRIORITY_CS1, None + ) + self.assertTrue("reqid cannot be null." in context.exception) + + def test_response_request_is_none(self): + with self.assertRaises(ValueError) as context: + UAttributesBuilder.response( + None + ) + self.assertTrue("request cannot be null." in context.exception) if __name__ == "__main__": unittest.main() diff --git a/tests/test_transport/test_builder/test_upayloadbuilder.py b/tests/test_transport/test_builder/test_upayloadbuilder.py index 18e68f6..5f56f0c 100644 --- a/tests/test_transport/test_builder/test_upayloadbuilder.py +++ b/tests/test_transport/test_builder/test_upayloadbuilder.py @@ -159,6 +159,24 @@ def test_unpack_given_upayload_proto_returns_method(self): self.assertEqual(original_msg, unpacked_msg) + def test_unpack_exception(self): + builder = self._create_upayload_builder() + + original_msg: Message = Method( + name="name", + request_type_url="request_type_url", + response_type_url="response_type_url", + request_streaming=None, + ) + upayload: UPayload = UPayload( + format=UPayloadFormat.UPAYLOAD_FORMAT_PROTOBUF, + value=b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", + ) + + unpacked_msg: Method = builder.unpack(upayload, Method) + + self.assertEqual(unpacked_msg, None) + def test_unpack_given_upayload_proto_returns_any(self): builder = self._create_upayload_builder() diff --git a/tests/test_transport/test_validate/test_uattributesvalidator.py b/tests/test_transport/test_validate/test_uattributesvalidator.py index a7768e6..0b369e9 100644 --- a/tests/test_transport/test_validate/test_uattributesvalidator.py +++ b/tests/test_transport/test_validate/test_uattributesvalidator.py @@ -26,10 +26,12 @@ import time import unittest +import uuid from uprotocol.proto.uattributes_pb2 import UPriority from uprotocol.proto.uri_pb2 import UUri, UAuthority, UEntity from uprotocol.proto.uuid_pb2 import UUID +from uprotocol.proto.uattributes_pb2 import UAttributes from uprotocol.transport.builder.uattributesbuilder import UAttributesBuilder from uprotocol.transport.validate.uattributesvalidator import ( UAttributesValidator, @@ -59,7 +61,7 @@ def build_source(): class TestUAttributesValidator(unittest.TestCase): - def test_fetching_validator_for_valid_types(self): + def test_fetching_validator_for_publish_type(self): publish = UAttributesValidator.get_validator( UAttributesBuilder.publish( build_source(), UPriority.UPRIORITY_CS0 @@ -67,6 +69,7 @@ def test_fetching_validator_for_valid_types(self): ) self.assertEqual("UAttributesValidator.Publish", str(publish)) + def test_fetching_validator_for_request_type(self): request = UAttributesValidator.get_validator( UAttributesBuilder.request( build_source(), UUri(), UPriority.UPRIORITY_CS4, 1000 @@ -74,6 +77,7 @@ def test_fetching_validator_for_valid_types(self): ) self.assertEqual("UAttributesValidator.Request", str(request)) + def test_fetching_validator_for_response_type(self): response = UAttributesValidator.get_validator( UAttributesBuilder.response( build_source(), @@ -84,6 +88,18 @@ def test_fetching_validator_for_valid_types(self): ) self.assertEqual("UAttributesValidator.Response", str(response)) + def test_fetching_validator_for_notification_type(self): + response = UAttributesValidator.get_validator( + UAttributesBuilder.notification( + build_source(), UUri(), UPriority.UPRIORITY_CS4 + ).build() + ) + self.assertEqual("UAttributesValidator.Notification", str(response)) + + def test_fetching_validator_for_no_type(self): + response = UAttributesValidator.get_validator(UAttributes()) + self.assertEqual("UAttributesValidator.Publish", str(response)) + def test_validate_uAttributes_for_publish_message_payload(self): attributes = UAttributesBuilder.publish( build_source(), UPriority.UPRIORITY_CS0 @@ -461,12 +477,14 @@ def test_validating_valid_sink_attribute(self): self.assertEqual(ValidationResult.success(), status) # def test_validating_invalid_ReqId_attribute(self): - # uuid_java = java.util.UUID.randomUUID() - # - # attributes = UAttributesBuilder.publish(UPriority.UPRIORITY_CS0).with_req_id( - # UUID.newBuilder().setMsb(uuid_java.getMostSignificantBits()).setLsb(uuid_java.getLeastSignificantBits()) - # .build()).build() - # + # uuid_python = uuid.UUID('12345678123456781234567812345678') + + # attributes = ( + # UAttributesBuilder.publish(build_source(), UPriority.UPRIORITY_CS0) + # .withReqId(None) + # .build() + # ) + # validator = Validators.PUBLISH.validator() # status = validator.validate_req_id(attributes) # self.assertTrue(status.is_failure()) @@ -483,6 +501,47 @@ def test_validating_valid_ReqId_attribute(self): status = validator.validate_req_id(attributes) self.assertEqual(ValidationResult.success(), status) + def test_validating_valid_type_attribute(self): + attributes = ( + UAttributesBuilder.notification(build_source(), build_sink(), UPriority.UPRIORITY_CS0) + .withReqId(Factories.UPROTOCOL.create()) + .build() + ) + + validator = Validators.NOTIFICATION.validator() + status = validator.validate_type(attributes) + self.assertEqual(ValidationResult.success(), status) + + def test_validating_valid_sink_attribute(self): + attributes = ( + UAttributesBuilder.notification(build_source(), build_sink(), UPriority.UPRIORITY_CS0) + .withReqId(Factories.UPROTOCOL.create()) + .build() + ) + + validator = Validators.NOTIFICATION.validator() + status = validator.validate_sink(attributes) + self.assertEqual(ValidationResult.success(), status) + + def test_validating_none_attribute(self): + attributes = None + + validator = Validators.NOTIFICATION.validator() + status = validator.validate_sink(attributes) + self.assertEqual("UAttributes cannot be null.", status.get_message()) + + def test_validating_invalid_sink_attribute(self): + attributes = ( + UAttributesBuilder.notification(build_source(), UUri(), UPriority.UPRIORITY_CS0) + .withReqId(Factories.UPROTOCOL.create()) + .build() + ) + + validator = Validators.NOTIFICATION.validator() + status = validator.validate_sink(attributes) + self.assertEqual("Missing Sink", status.get_message()) + + def test_validating_invalid_PermissionLevel_attribute(self): with self.assertRaises(ValueError) as context: UAttributesBuilder.publish( diff --git a/tests/test_uri/test_serializer/test_microuriserializer.py b/tests/test_uri/test_serializer/test_microuriserializer.py index 0f93994..44af680 100644 --- a/tests/test_uri/test_serializer/test_microuriserializer.py +++ b/tests/test_uri/test_serializer/test_microuriserializer.py @@ -88,6 +88,12 @@ def test_serialize_good_ipv4_based_authority(self): self.assertEqual(str(uri), str(uri2)) self.assertTrue(uri == uri2) + def test_serialize_bad_authority(self): + uri = UUri(authority=UAuthority(ip=b"123456789"), + entity=UEntity(id=29999, version_major=254), resource=UResourceBuilder.for_rpc_request(99)) + bytes_uuri = MicroUriSerializer().serialize(uri) + self.assertEqual(bytes_uuri, bytearray()) + def test_serialize_good_ipv6_based_authority(self): uri = UUri(authority=UAuthority( ip=socket.inet_pton(socket.AF_INET6, "2001:0db8:85a3:0000:0000:8a2e:0370:7334")), diff --git a/tests/test_uri/test_serializer/test_shorturiserializer.py b/tests/test_uri/test_serializer/test_shorturiserializer.py index 1604008..76d41d4 100644 --- a/tests/test_uri/test_serializer/test_shorturiserializer.py +++ b/tests/test_uri/test_serializer/test_shorturiserializer.py @@ -89,6 +89,10 @@ def test_short_deserialize_empty_uri(self): uri = ShortUriSerializer().deserialize("") self.assertEqual(uri, UUri()) + def test_short_deserialize_uri_too_short(self): + uri = ShortUriSerializer().deserialize("1") + self.assertEqual(uri, UUri()) + def test_short_deserialize_uri_with_scheme_and_authority(self): uri = ShortUriSerializer().deserialize("up://mypc/1/1/1") self.assertTrue(uri.authority is not None) @@ -235,6 +239,11 @@ def test_short_deserialize_with_remote_authority_and_invalid_characters_for_reso uri = ShortUriSerializer().deserialize("//mypc/1/1/abc") self.assertEqual(uri.resource, UResource()) + def test_parse_from_string_none(self): + with self.assertRaises(ValueError) as context: + ShortUriSerializer().parse_from_string(None) + self.assertTrue(" Resource must have a command name" in context.exception) + if __name__ == "__main__": unittest.main() diff --git a/uprotocol/cloudevent/factory/ucloudevent.py b/uprotocol/cloudevent/factory/ucloudevent.py index 154864b..4c2046e 100644 --- a/uprotocol/cloudevent/factory/ucloudevent.py +++ b/uprotocol/cloudevent/factory/ucloudevent.py @@ -25,7 +25,8 @@ import time -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone +import copy from cloudevents.http import CloudEvent from google.protobuf import any_pb2 @@ -271,8 +272,9 @@ def add_communication_status( """ if communication_status is None: return ce - ce.__setitem__("commstatus", communication_status) - return ce + ce_new = copy.deepcopy(ce) + ce_new.__setitem__("commstatus", communication_status) + return ce_new @staticmethod def get_creation_timestamp(ce: CloudEvent) -> int: @@ -289,7 +291,7 @@ def get_creation_timestamp(ce: CloudEvent) -> int: ) uuid = LongUuidSerializer.instance().deserialize(cloud_event_id) - return UUIDUtils.getTime(uuid) if uuid is not None else None + return UUIDUtils.get_time(uuid) if uuid is not None else None @staticmethod def is_expired_by_cloud_event_creation_date(ce: CloudEvent) -> bool: @@ -313,8 +315,8 @@ def is_expired_by_cloud_event_creation_date(ce: CloudEvent) -> bool: if cloud_event_creation_time is None: return False - now = datetime.now() - creation_time_plus_ttl = cloud_event_creation_time + timedelta( + now = datetime.now(timezone.utc) + creation_time_plus_ttl = datetime.fromisoformat(cloud_event_creation_time) + timedelta( milliseconds=maybe_ttl ) diff --git a/uprotocol/transport/builder/uattributesbuilder.py b/uprotocol/transport/builder/uattributesbuilder.py index b33833d..00d18db 100644 --- a/uprotocol/transport/builder/uattributesbuilder.py +++ b/uprotocol/transport/builder/uattributesbuilder.py @@ -25,6 +25,7 @@ # ------------------------------------------------------------------------- from multimethod import multimethod +from typing import Union from uprotocol.proto.uattributes_pb2 import ( UAttributes, @@ -39,10 +40,12 @@ class UAttributesBuilder: """ - Construct the UAttributesBuilder with the configurations that are required for every payload transport. + Construct the UAttributesBuilder with the configurations that are + required for every payload transport. @param id Unique identifier for the message. - @param type Message type such as Publish a state change, RPC request or RPC response. + @param type Message type such as Publish a state change, + RPC request or RPC response. @param priority uProtocol Prioritization classifications. """ @@ -87,7 +90,8 @@ def notification(source: UUri, sink: UUri, priority: UPriority): @param source Source address of the message. @param sink The destination URI. @param priority The priority of the message. - @return Returns the UAttributesBuilder with the configured source, priority and sink. + @return Returns the UAttributesBuilder with the configured source, + priority and sink. """ if source is None: raise ValueError("Source cannot be None.") @@ -110,7 +114,8 @@ def request(source: UUri, sink: UUri, priority: UPriority, ttl: int): @param sink The destination URI. @param priority The priority of the message. @param ttl The time to live in milliseconds. - @return Returns the UAttributesBuilder with the configured priority, sink and ttl. + @return Returns the UAttributesBuilder with the configured + priority, sink and ttl. """ if source is None: raise ValueError("Source cannot be None.") @@ -133,14 +138,21 @@ def request(source: UUri, sink: UUri, priority: UPriority, ttl: int): ) @multimethod - def response(source: UUri, sink: UUri, priority: int, reqid: UUID): + def response( + source: Union[UUri, None], + sink: Union[UUri, None], + priority: Union[int, None], + reqid: Union[UUID, None], + ): """ Construct a UAttributesBuilder for a response message. @param source Source address of the message. @param sink The destination URI. @param priority The priority of the message. - @param reqid The original request UUID used to correlate the response to the request. - @return Returns the UAttributesBuilder with the configured priority, sink and reqid. + @param reqid The original request UUID used to correlate the + response to the request. + @return Returns the UAttributesBuilder with the configured priority, + sink and reqid. """ if priority is None: raise ValueError("UPriority cannot be null.") @@ -161,7 +173,7 @@ def response(source: UUri, sink: UUri, priority: int, reqid: UUID): ) @multimethod - def response(request: UAttributes): + def response(request: Union[UAttributes, None]): if request is None: raise ValueError("request cannot be null.") return UAttributesBuilder.response( @@ -233,7 +245,8 @@ def withTraceparent(self, traceparent: str): Add the traceparent. @param reqid the traceparent. - @return Returns the UAttributesBuilder with the configured traceparent. + @return Returns the UAttributesBuilder with the configured + traceparent. """ self.traceparent = traceparent return self diff --git a/uprotocol/transport/validate/uattributesvalidator.py b/uprotocol/transport/validate/uattributesvalidator.py index 82f17fc..a0f2b26 100644 --- a/uprotocol/transport/validate/uattributesvalidator.py +++ b/uprotocol/transport/validate/uattributesvalidator.py @@ -135,10 +135,7 @@ def validate_ttl(attr: UAttributes) -> ValidationResult: @return:Returns a ValidationResult that is success or failed with a failure message. """ - if attr.HasField("ttl") and attr.ttl <= 0: - return ValidationResult.failure(f"Invalid TTL [{attr.ttl}]") - else: - return ValidationResult.success() + return ValidationResult.success() @staticmethod def validate_sink(attr: UAttributes) -> ValidationResult: @@ -300,10 +297,6 @@ def validate_ttl(self, attributes_value: UAttributes) -> ValidationResult: """ if not attributes_value.HasField("ttl"): return ValidationResult.failure("Missing TTL") - if attributes_value.ttl <= 0: - return ValidationResult.failure( - f"Invalid TTL [{attributes_value.ttl}]" - ) return ValidationResult.success() From 35576acae922060030e2312f865017e2a24acd7b Mon Sep 17 00:00:00 2001 From: Matthew D'Alonzo Date: Tue, 9 Apr 2024 11:59:35 -0400 Subject: [PATCH 4/6] Update pyproject.toml Current system does not support tool.coverage.report, so I have removed it for now. Those exclusion cases will be covered in the coverage.rc file --- pyproject.toml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index b8e76f4..1c22c14 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,10 +26,4 @@ coverage = "^5.5" [build-system] requires = ["poetry-core"] -build-backend = "poetry.core.masonry.api" - -[tool.coverage.report] -exclude_also = [ - "pass", - "raise NotImplementedError" - ] \ No newline at end of file +build-backend = "poetry.core.masonry.api" \ No newline at end of file From 43f004959c8fe40c54ce9882c309a198b7dc8fd8 Mon Sep 17 00:00:00 2001 From: Neelam Kushwah Date: Tue, 9 Apr 2024 13:45:52 -0400 Subject: [PATCH 5/6] Add steps to comment on the PR with the coverage report --- .github/workflows/coverage.yml | 147 +++++++++++++++++---------------- 1 file changed, 75 insertions(+), 72 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index fda24a0..1455f5c 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -1,7 +1,8 @@ name: Python Test and Coverage on: - pull_request: + pull_request_target: + types: [opened, synchronize, reopened] branches: - main @@ -10,77 +11,79 @@ jobs: test-and-coverage: name: Test with coverage runs-on: ubuntu-latest -# permissions: -# contents: write -# pull-requests: write -# repository-projects: write -# id-token: write + permissions: + pull-requests: write steps: - - name: Checkout code - uses: actions/checkout@v3 - - - name: Set up Apache Maven Central - uses: actions/setup-java@v3 - with: # configure settings.xml - distribution: 'temurin' - java-version: '11' - server-id: ossrh - server-username: OSSRH_USER - server-password: OSSRH_PASSWORD - - - name: Set up Python - uses: actions/setup-python@v3 - with: - python-version: '3.x' - - - name: Install Poetry - run: | - python -m pip install --upgrade pip - python -m pip install poetry - - - name: Install dependencies - run: | - poetry install - - - name: Run prebuild script - run: | - cd scripts - # Run the script within the Poetry virtual environment - poetry run python pull_and_compile_protos.py - - - name: Run tests with coverage - run: | - poetry run coverage run --source=uprotocol --omit=uprotocol/proto/*,uprotocol/cloudevent/*_pb2.py,tests/*,*/__init__.py -m pytest - poetry run coverage report > coverage_report.txt - export COVERAGE_PERCENTAGE=$(awk '/TOTAL/{print $4}' coverage_report.txt) - echo "COVERAGE_PERCENTAGE=$COVERAGE_PERCENTAGE" >> $GITHUB_ENV - echo "COVERAGE_PERCENTAGE: $COVERAGE_PERCENTAGE" - poetry run coverage html - - - - name: Upload coverage report - uses: actions/upload-artifact@v2 - with: - name: coverage-report - path: htmlcov/ - -# - name: Comment PR with coverage results -# uses: actions/github-script@v6 -# with: -# script: | -# const COVERAGE_PERCENTAGE = process.env.COVERAGE_PERCENTAGE;; -# const COVERAGE_REPORT_PATH = `https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}/`; -# -# github.rest.issues.createComment({ -# issue_number: context.issue.number, -# owner: context.repo.owner, -# repo: context.repo.repo, -# body: ` -# Code coverage report is ready! :chart_with_upwards_trend: -# -# - **Code Coverage Percentage:** ${COVERAGE_PERCENTAGE} -# - **Code Coverage Report:** [View Coverage Report](${COVERAGE_REPORT_PATH}) -# ` -# }); + - run: | + git config --global user.name 'eclipse-uprotocol-bot' + git config --global user.email 'uprotocol-bot@eclipse.org' + + - name: Checkout code + uses: actions/checkout@v3 + + - name: Set up Apache Maven Central + uses: actions/setup-java@v3 + with: # configure settings.xml + distribution: 'temurin' + java-version: '11' + server-id: ossrh + server-username: OSSRH_USER + server-password: OSSRH_PASSWORD + + - name: Set up Python + uses: actions/setup-python@v3 + with: + python-version: '3.x' + + - name: Install Poetry + run: | + python -m pip install --upgrade pip + python -m pip install poetry + + - name: Install dependencies + run: | + poetry install + + - name: Run prebuild script + run: | + cd scripts + # Run the script within the Poetry virtual environment + poetry run python pull_and_compile_protos.py + + - name: Run tests with coverage + run: | + poetry run coverage run --source=uprotocol --omit=uprotocol/proto/*,uprotocol/cloudevent/*_pb2.py,tests/*,*/__init__.py -m pytest + poetry run coverage report > coverage_report.txt + export COVERAGE_PERCENTAGE=$(awk '/TOTAL/{print $4}' coverage_report.txt) + echo "COVERAGE_PERCENTAGE=$COVERAGE_PERCENTAGE" >> $GITHUB_ENV + echo "COVERAGE_PERCENTAGE: $COVERAGE_PERCENTAGE" + poetry run coverage html + + + - name: Upload coverage report + uses: actions/upload-artifact@v2 + with: + name: coverage-report + path: htmlcov/ + + - name: Comment PR with coverage results + uses: actions/github-script@v6 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + + script: | + const COVERAGE_PERCENTAGE = process.env.COVERAGE_PERCENTAGE;; + const COVERAGE_REPORT_PATH = `https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}/`; + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: ` + Code coverage report is ready! :chart_with_upwards_trend: + + - **Code Coverage Percentage:** ${COVERAGE_PERCENTAGE} + - **Code Coverage Report:** [View Coverage Report](${COVERAGE_REPORT_PATH}) + ` + }); From b59df370d767b17477964373d77cc73538ce934d Mon Sep 17 00:00:00 2001 From: Neelam Kushwah Date: Wed, 10 Apr 2024 16:55:22 -0400 Subject: [PATCH 6/6] Remove comment on PR step --- .github/workflows/coverage.yml | 50 +++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 1455f5c..764167b 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -1,8 +1,7 @@ name: Python Test and Coverage on: - pull_request_target: - types: [opened, synchronize, reopened] + pull_request: branches: - main @@ -67,23 +66,36 @@ jobs: name: coverage-report path: htmlcov/ - - name: Comment PR with coverage results + - name: Check code coverage uses: actions/github-script@v6 with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - const COVERAGE_PERCENTAGE = process.env.COVERAGE_PERCENTAGE;; - const COVERAGE_REPORT_PATH = `https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}/`; - github.rest.issues.createComment({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - body: ` - Code coverage report is ready! :chart_with_upwards_trend: - - - **Code Coverage Percentage:** ${COVERAGE_PERCENTAGE} - - **Code Coverage Report:** [View Coverage Report](${COVERAGE_REPORT_PATH}) - ` - }); - + const COVERAGE_PERCENTAGE = process.env.COVERAGE_PERCENTAGE; + if (parseInt(COVERAGE_PERCENTAGE) < 95){ + core.setFailed(`Coverage Percentage is less than 95%: ${COVERAGE_PERCENTAGE}`); + }else{ + core.info(`Success`); + core.info(parseInt(COVERAGE_PERCENTAGE)); + } + + +# - name: Comment PR with coverage results +# uses: actions/github-script@v6 +# with: +# github-token: ${{ secrets.GITHUB_TOKEN }} +# +# script: | +# const COVERAGE_PERCENTAGE = process.env.COVERAGE_PERCENTAGE;; +# const COVERAGE_REPORT_PATH = `https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}/`; +# github.rest.issues.createComment({ +# issue_number: context.issue.number, +# owner: context.repo.owner, +# repo: context.repo.repo, +# body: ` +# Code coverage report is ready! :chart_with_upwards_trend: +# +# - **Code Coverage Percentage:** ${COVERAGE_PERCENTAGE} +# - **Code Coverage Report:** [View Coverage Report](${COVERAGE_REPORT_PATH}) +# ` +# }); +#