add more tests, simplify tests

tests/cfdp/test_src_handler.py

@@ -47,6 +47,7 @@ def common_setup(
         self.cfdp_user.transaction_indication = MagicMock()
         self.cfdp_user.transaction_finished_indication = MagicMock()
         self.seq_num_provider = SeqCountProvider(bit_width=8)
+        self.expected_cfdp_state = CfdpState.BUSY_CLASS_1_NACKED
         self.source_id = ByteFieldU16(1)
         self.dest_id = ByteFieldU16(2)
         self.file_path = Path(f"{tempfile.gettempdir()}/hello.txt")
@@ -70,7 +71,7 @@ def common_setup(
         )
 
     def _common_empty_file_test(
-        self, transmission_mode: Optional[TransmissionMode], expected_state: CfdpState
+        self, transmission_mode: Optional[TransmissionMode]
     ) -> EofPdu:
         dest_path = Path("/tmp/hello_copy.txt")
         dest_id = ByteFieldU16(2)
@@ -83,11 +84,9 @@ def _common_empty_file_test(
             trans_mode=transmission_mode,
             closure_requested=None,
         )
-        transaction_id = self._start_source_transaction(
-            dest_id, put_req, expected_state
-        )
+        transaction_id = self._start_source_transaction(dest_id, put_req)
         fsm_res = self.source_handler.state_machine()
-        self._state_checker(fsm_res, True, expected_state, TransactionStep.SENDING_EOF)
+        self._state_checker(fsm_res, True, TransactionStep.SENDING_EOF)
         self.assertEqual(self.source_handler.transaction_seq_num.value, 3)
         next_packet = self.source_handler.get_next_packet()
         self.assertIsNotNone(next_packet)
@@ -107,8 +106,11 @@ def _common_empty_file_test(
         return eof_pdu
 
     def _common_small_file_test(
-        self, closure_requested: bool, file_content: str, expected_state: CfdpState
-    ) -> TransactionId:
+        self,
+        transmission_mode: Optional[TransmissionMode],
+        closure_requested: bool,
+        file_content: str,
+    ) -> Tuple[EofPdu, TransactionId]:
         dest_path = Path("/tmp/hello_copy.txt")
         self.source_id = ByteFieldU32(1)
         self.dest_id = ByteFieldU32(2)
@@ -119,7 +121,7 @@ def _common_small_file_test(
             source_file=self.file_path,
             dest_file=dest_path,
             # Let the transmission mode be auto-determined by the remote MIB
-            trans_mode=None,
+            trans_mode=transmission_mode,
             closure_requested=closure_requested,
         )
         with open(self.file_path, "wb") as of:
@@ -129,9 +131,7 @@ def _common_small_file_test(
             crc32 = crc32.digest()
             of.write(data)
         file_size = self.file_path.stat().st_size
-        transaction_id = self._start_source_transaction(
-            self.dest_id, put_req, expected_state
-        )
+        transaction_id = self._start_source_transaction(self.dest_id, put_req)
         self.assertEqual(transaction_id.source_id, self.source_handler.source_id)
         self.assertEqual(transaction_id.seq_num.value, 2)
         self.assertEqual(self.source_handler.transaction_seq_num.value, 2)
@@ -143,23 +143,23 @@ def _common_small_file_test(
         file_data_pdu = self._check_fsm_and_contained_file_data(fsm_res, next_packet)
         self.assertFalse(file_data_pdu.has_segment_metadata)
         self.assertEqual(file_data_pdu.transaction_seq_num.value, 2)
-        self.assertEqual(file_data_pdu.file_data, "Hello World\n".encode())
+        self.assertEqual(file_data_pdu.file_data, file_content.encode())
         self.assertEqual(file_data_pdu.offset, 0)
         fsm_res = self.source_handler.state_machine()
-        self._state_checker(
-            fsm_res, True, CfdpState.BUSY_CLASS_1_NACKED, TransactionStep.SENDING_EOF
-        )
+        self._state_checker(fsm_res, True, TransactionStep.SENDING_EOF)
         next_packet = self.source_handler.get_next_packet()
         assert next_packet is not None
         self.assertEqual(next_packet.pdu_directive_type, DirectiveType.EOF_PDU)
         eof_pdu = next_packet.to_eof_pdu()
         self.assertEqual(crc32, eof_pdu.file_checksum)
         self.assertEqual(eof_pdu.file_size, file_size)
         self.assertEqual(eof_pdu.condition_code, ConditionCode.NO_ERROR)
-        return transaction_id
+        fsm_res = self.source_handler.state_machine()
+        self._verify_eof_indication(transaction_id)
+        return eof_pdu, transaction_id
 
     def _transaction_with_file_data_wrapper(
-        self, dest_path: Path, data: bytes, expected_state: CfdpState
+        self, dest_path: Path, data: bytes
     ) -> Tuple[TransactionId, int, bytes]:
         put_req = PutRequest(
             destination_id=self.dest_id,
@@ -176,9 +176,7 @@ def _transaction_with_file_data_wrapper(
             of.write(data)
         file_size = self.file_path.stat().st_size
         self.local_cfg.local_entity_id = self.source_id
-        transaction_id = self._start_source_transaction(
-            self.dest_id, put_req, expected_state
-        )
+        transaction_id = self._start_source_transaction(self.dest_id, put_req)
         return transaction_id, file_size, crc32
 
     def _first_file_segment_handling(
@@ -203,7 +201,6 @@ def _check_fsm_and_contained_file_data(
         self._state_checker(
             fsm_res,
             False,
-            CfdpState.BUSY_CLASS_1_NACKED,
             TransactionStep.SENDING_FILE_DATA,
         )
         self.assertFalse(pdu_holder.is_file_directive)
@@ -213,15 +210,13 @@ def _start_source_transaction(
         self,
         dest_id: UnsignedByteField,
         put_request: PutRequest,
-        expected_state: CfdpState,
     ) -> TransactionId:
         self.remote_cfg.entity_id = dest_id
         self.source_handler.put_request(put_request, self.remote_cfg)
         fsm_res = self.source_handler.state_machine()
         self._state_checker(
             fsm_res,
             True,
-            expected_state,
             TransactionStep.SENDING_METADATA,
         )
         transaction_id = self.source_handler.transaction_id
@@ -256,16 +251,15 @@ def _state_checker(
         self,
         fsm_res: Optional[FsmResult],
         packet_ready: bool,
-        expected_state: CfdpState,
         expected_step: TransactionStep,
     ):
         if fsm_res is not None:
-            self.assertEqual(fsm_res.states.state, expected_state)
+            self.assertEqual(fsm_res.states.state, self.expected_cfdp_state)
             self.assertEqual(fsm_res.states.step, expected_step)
-        self.assertEqual(self.source_handler.states.state, expected_state)
+        self.assertEqual(self.source_handler.states.state, self.expected_cfdp_state)
         self.assertEqual(self.source_handler.states.step, expected_step)
         self.assertEqual(self.source_handler.states.packets_ready, packet_ready)
-        self.assertEqual(self.source_handler.state, expected_state)
+        self.assertEqual(self.source_handler.state, self.expected_cfdp_state)
         self.assertEqual(self.source_handler.step, expected_step)
         self.assertEqual(self.source_handler.packets_ready, packet_ready)
 

tests/cfdp/test_src_handler_acked.py

@@ -8,6 +8,7 @@
 from spacepackets.cfdp.pdu import (
     AckPdu,
     DeliveryCode,
+    EofPdu,
     FileDeliveryStatus,
     FinishedPdu,
     TransactionStatus,
@@ -22,18 +23,14 @@
 class TestSourceHandlerAcked(TestCfdpSourceHandler):
     def setUp(self) -> None:
         self.common_setup(True, TransmissionMode.ACKNOWLEDGED)
+        self.expected_cfdp_state = CfdpState.BUSY_CLASS_2_ACKED
 
-    def test_empty_file_transfer(self):
-        eof_pdu = self._common_empty_file_test(None, CfdpState.BUSY_CLASS_2_ACKED)
+    def _generic_success_ack_handling(self, eof_pdu: EofPdu):
         self._state_checker(
             None,
             False,
-            CfdpState.BUSY_CLASS_2_ACKED,
             TransactionStep.WAITING_FOR_EOF_ACK,
         )
-        # TODO: 1: Acknowledge EOF PDU by inserting ACK, 2: Insert Finished PDU, 3: Retrieve
-        # and check ACK PDU generated as a response to the Finished PDU.
-        # pdu_conf = PduConfig(eof_pdu.source_entity_id, eof_pdu.dest_entity_id, eof_pdu.transaction_seq_num, eof_pdu.transmission_mode)
         pdu_conf = eof_pdu.pdu_header.pdu_conf
         ack_pdu = AckPdu(
             pdu_conf,
@@ -47,7 +44,6 @@ def test_empty_file_transfer(self):
         self._state_checker(
             None,
             False,
-            CfdpState.BUSY_CLASS_2_ACKED,
             TransactionStep.WAITING_FOR_FINISHED,
         )
         finished_pdu = FinishedPdu(
@@ -63,7 +59,6 @@ def test_empty_file_transfer(self):
         self._state_checker(
             None,
             True,
-            CfdpState.BUSY_CLASS_2_ACKED,
             TransactionStep.SENDING_ACK_OF_FINISHED,
         )
         next_pdu = self.source_handler.get_next_packet()
@@ -80,9 +75,21 @@ def test_empty_file_transfer(self):
         pdu_conf.direction = Direction.TOWARDS_RECEIVER
         self.assertEqual(ack_pdu.pdu_header.pdu_conf, pdu_conf)
         self.source_handler.state_machine()
+        self.expected_cfdp_state = CfdpState.IDLE
         self._state_checker(
             None,
             False,
-            CfdpState.IDLE,
             TransactionStep.IDLE,
         )
+
+    def test_empty_file_transfer(self):
+        eof_pdu = self._common_empty_file_test(None)
+        self._generic_success_ack_handling(eof_pdu)
+
+    def test_small_file_transfer(self):
+        eof_pdu, _ = self._common_small_file_test(
+            TransmissionMode.ACKNOWLEDGED,
+            True,
+            "Hello World!",
+        )
+        self._generic_success_ack_handling(eof_pdu)

tests/cfdp/test_src_handler_nak_closure.py

@@ -30,39 +30,38 @@ def setUp(self) -> None:
         self.seq_num_provider.get_and_increment = MagicMock(return_value=2)
 
     def test_empty_file_pdu_generation_nacked_by_remote_cfg(self):
-        self._common_empty_file_test(None, CfdpState.BUSY_CLASS_1_NACKED)
+        self._common_empty_file_test(None)
         self._pass_simple_finish_pdu_to_source_handler()
         # Transaction should be finished
         fsm_res = self.source_handler.state_machine()
-        self._state_checker(fsm_res, False, CfdpState.IDLE, TransactionStep.IDLE)
+        self.expected_cfdp_state = CfdpState.IDLE
+        self._state_checker(fsm_res, False, TransactionStep.IDLE)
 
     def test_empty_file_pdu_generation_nacked_explicitely(self):
         self.remote_cfg.default_transmission_mode = TransmissionMode.ACKNOWLEDGED
-        self._common_empty_file_test(
-            TransmissionMode.UNACKNOWLEDGED, CfdpState.BUSY_CLASS_1_NACKED
-        )
+        self._common_empty_file_test(TransmissionMode.UNACKNOWLEDGED)
         self._pass_simple_finish_pdu_to_source_handler()
         # Transaction should be finished
         fsm_res = self.source_handler.state_machine()
-        self._state_checker(fsm_res, False, CfdpState.IDLE, TransactionStep.IDLE)
+        self.expected_cfdp_state = CfdpState.IDLE
+        self._state_checker(fsm_res, False, TransactionStep.IDLE)
 
     def test_small_file_pdu_generation(self):
         file_content = "Hello World\n"
-        transaction_id = self._common_small_file_test(
-            True, file_content, CfdpState.BUSY_CLASS_1_NACKED
+        _, transaction_id = self._common_small_file_test(
+            TransmissionMode.UNACKNOWLEDGED, True, file_content
         )
         self._verify_eof_indication(transaction_id)
         self.source_handler.state_machine()
         self._pass_simple_finish_pdu_to_source_handler()
         # Transaction should be finished
         fsm_res = self.source_handler.state_machine()
-        self._state_checker(fsm_res, False, CfdpState.IDLE, TransactionStep.IDLE)
+        self.expected_cfdp_state = CfdpState.IDLE
+        self._state_checker(fsm_res, False, TransactionStep.IDLE)
 
     def test_invalid_dir_pdu_passed(self):
         dest_id = ByteFieldU16(2)
-        self._start_source_transaction(
-            dest_id, self._prepare_dummy_put_req(dest_id), CfdpState.BUSY_CLASS_1_NACKED
-        )
+        self._start_source_transaction(dest_id, self._prepare_dummy_put_req(dest_id))
         finish_pdu = self._prepare_finish_pdu()
         finish_pdu.pdu_file_directive.pdu_header.direction = Direction.TOWARDS_RECEIVER
         with self.assertRaises(InvalidPduDirection):
@@ -82,7 +81,6 @@ def test_cancelled_transaction(self):
         transaction_id, file_size, crc32 = self._transaction_with_file_data_wrapper(
             dest_path,
             rand_data[0 : self.file_segment_len],
-            CfdpState.BUSY_CLASS_1_NACKED,
         )
         self._first_file_segment_handling(self.source_handler, rand_data)
         self.assertTrue(self.source_handler.cancel_request(transaction_id))
@@ -96,7 +94,8 @@ def test_cancelled_transaction(self):
         self.assertEqual(eof_pdu.file_size, self.file_segment_len)
         self.assertEqual(eof_pdu.file_size, file_size)
         fsm_res = self.source_handler.state_machine()
-        self._state_checker(fsm_res, False, CfdpState.IDLE, TransactionStep.IDLE)
+        self.expected_cfdp_state = CfdpState.IDLE
+        self._state_checker(fsm_res, False, TransactionStep.IDLE)
 
     def test_invalid_source_id_pdu_passed(self):
         dest_id = ByteFieldU16(2)
@@ -119,9 +118,7 @@ def test_invalid_dest_id_pdu_passed(self):
         self.assertEqual(exception.expected_dest_id, ByteFieldU16(3))
 
     def _regular_transaction_start(self, dest_id: UnsignedByteField) -> FinishedPdu:
-        self._start_source_transaction(
-            dest_id, self._prepare_dummy_put_req(dest_id), CfdpState.BUSY_CLASS_1_NACKED
-        )
+        self._start_source_transaction(dest_id, self._prepare_dummy_put_req(dest_id))
         finish_pdu = self._prepare_finish_pdu()
         self.assertEqual(finish_pdu.transaction_seq_num.value, 2)
         return finish_pdu
@@ -140,7 +137,6 @@ def _pass_simple_finish_pdu_to_source_handler(self):
         self._state_checker(
             None,
             False,
-            CfdpState.BUSY_CLASS_1_NACKED,
             TransactionStep.WAITING_FOR_FINISHED,
         )
         self.source_handler.insert_packet(self._prepare_finish_pdu())

tests/cfdp/test_src_handler_nak_no_closure.py

@@ -27,32 +27,31 @@ def setUp(self) -> None:
 
     def test_empty_file_nacked_by_def_config(self):
         self._common_empty_file_test(
-            transmission_mode=None, expected_state=CfdpState.BUSY_CLASS_1_NACKED
+            transmission_mode=None,
         )
         fsm_res = self.source_handler.state_machine()
         self.cfdp_user.transaction_finished_indication.assert_called_once()
         self.assertEqual(self.cfdp_user.transaction_finished_indication.call_count, 1)
         self.source_handler.state_machine()
-        self.assertEqual(fsm_res.states.state, CfdpState.IDLE)
-        self.assertEqual(fsm_res.states.step, TransactionStep.IDLE)
+        self.expected_cfdp_state = CfdpState.IDLE
+        self._state_checker(fsm_res, False, TransactionStep.IDLE)
 
     def test_empty_file_explicit_nacked(self):
         self.remote_cfg.default_transmission_mode = TransmissionMode.ACKNOWLEDGED
         self._common_empty_file_test(
             transmission_mode=TransmissionMode.UNACKNOWLEDGED,
-            expected_state=CfdpState.BUSY_CLASS_1_NACKED,
         )
         fsm_res = self.source_handler.state_machine()
         self.cfdp_user.transaction_finished_indication.assert_called_once()
         self.assertEqual(self.cfdp_user.transaction_finished_indication.call_count, 1)
         self.source_handler.state_machine()
-        self.assertEqual(fsm_res.states.state, CfdpState.IDLE)
-        self.assertEqual(fsm_res.states.step, TransactionStep.IDLE)
+        self.expected_cfdp_state = CfdpState.IDLE
+        self._state_checker(fsm_res, False, TransactionStep.IDLE)
 
     def test_small_file_pdu_generation(self):
         file_content = "Hello World\n"
-        transaction_id = self._common_small_file_test(
-            False, file_content, CfdpState.BUSY_CLASS_1_NACKED
+        eof_pdu, transaction_id = self._common_small_file_test(
+            None, False, file_content
         )
         self._verify_eof_indication(transaction_id)
         self._test_transaction_completion()
@@ -68,7 +67,7 @@ def test_perfectly_segmented_file_pdu_generation(self):
         self.source_handler.source_id = self.source_id
         dest_path = Path("/tmp/hello_two_segments_copy.txt")
         transaction_id, file_size, crc32 = self._transaction_with_file_data_wrapper(
-            dest_path, rand_data, CfdpState.BUSY_CLASS_1_NACKED
+            dest_path, rand_data
         )
         self.assertEqual(transaction_id.source_id, self.source_id)
         self.assertEqual(transaction_id.seq_num.value, 0)
@@ -97,7 +96,7 @@ def test_segmented_file_pdu_generation(self):
         self.source_handler.source_id = self.source_id
         dest_path = Path("/tmp/hello_two_segments_imperfect_copy.txt")
         transaction_id, file_size, crc32 = self._transaction_with_file_data_wrapper(
-            dest_path, rand_data, CfdpState.BUSY_CLASS_1_NACKED
+            dest_path, rand_data
         )
         self.assertEqual(transaction_id.source_id, self.source_id)
         self.assertEqual(transaction_id.seq_num.value, 0)

tmtccmd/cfdp/handler/source.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 from collections import deque
 import enum
-import copy
 import logging
 from dataclasses import dataclass
 from typing import Deque, Optional, Tuple
@@ -701,17 +700,15 @@ def _setup_transmission_mode(self):
         assert self._put_req is not None
         assert self._params.remote_cfg is not None
         # Transmission mode settings in the put request override settings from the remote MIB
-        if self._put_req.trans_mode is not None:
-            trans_mode_to_set = self._put_req.trans_mode
-        else:
+        trans_mode_to_set = self._put_req.trans_mode
+        if trans_mode_to_set is None:
             trans_mode_to_set = self._params.remote_cfg.default_transmission_mode
-        self._params.transmission_mode = trans_mode_to_set
-        if self._put_req.closure_requested is not None:
-            closure_req_to_set = self._put_req.closure_requested
-        else:
+        closure_req_to_set = self._put_req.closure_requested
+        if closure_req_to_set is None:
             closure_req_to_set = self._params.remote_cfg.closure_requested
-        self._params.crc_helper.checksum_type = self._params.remote_cfg.crc_type
+        self._params.transmission_mode = trans_mode_to_set
         self._params.closure_requested = closure_req_to_set
+        self._params.crc_helper.checksum_type = self._params.remote_cfg.crc_type
 
     def _add_packet_to_be_sent(self, packet: GenericPduPacket):
         self._pdus_to_be_sent.append(PduHolder(packet))