Update Os File UTs (#3619)

* bugfixes to uts. the old way was generating out of range offsets

* bugfixes

* adding more helpful messages and bugfix to ut for when mode is OPEN_APPEND

* removed unnecessary prints

* ci fixes and updates based on review

* fix for static analyzer

* sp

* should appease ci

* append moves the file pointer before write not on open

* typo

* cleaned up code

* open with append will still have the state position to 0

Author: kevin-f-ortega

Os/test/ut/directory/DirectoryRules.cpp

@@ -22,6 +22,7 @@ bool Os::Test::Directory::Tester::Open::precondition(const Os::Test::Directory::
 
 void Os::Test::Directory::Tester::Open::action(Os::Test::Directory::Tester &state) {
     Os::Directory::OpenMode mode = STest::Pick::lowerUpper(0, 1) == 1 ? Os::Directory::READ : Os::Directory::CREATE_IF_MISSING;
+    printf("--> Rule: %s pathname %s mode %d\n", this->getName(), state.m_path.c_str(), mode);
     Os::Directory::Status status = state.m_directory.open(state.m_path.c_str(), mode);
     ASSERT_EQ(status, Os::Directory::Status::OP_OK);
     state.m_state = Os::Test::Directory::Tester::DirectoryState::OPEN;
@@ -40,6 +41,7 @@ bool Os::Test::Directory::Tester::OpenAlreadyExistsError::precondition(const Os:
 }
 
 void Os::Test::Directory::Tester::OpenAlreadyExistsError::action(Os::Test::Directory::Tester &state) {
+    printf("--> Rule: %s pathname %s\n", this->getName(), state.m_path.c_str());
     Os::Directory new_directory;
     Os::Directory::Status status = new_directory.open(state.m_path.c_str(), Os::Directory::OpenMode::CREATE_EXCLUSIVE);
     ASSERT_EQ(status, Os::Directory::Status::ALREADY_EXISTS);
@@ -100,8 +102,9 @@ bool Os::Test::Directory::Tester::ReadOneFile::precondition(const Os::Test::Dire
 }
 
 void Os::Test::Directory::Tester::ReadOneFile::action(Os::Test::Directory::Tester &state) {
+    printf("--> Rule: %s\n", this->getName());
     ASSERT_TRUE(state.m_directory.isOpen());
-    char filename[100];
+    char filename[100] = {0};
     Os::Directory::Status status = state.m_directory.read(filename, 100);
     // If seek is at the end of the directory, expect NO_MORE_FILES - otherwise expect normal read and valid filename
     if (state.m_seek_position < static_cast<FwIndexType>(state.m_filenames.size())) {

Os/test/ut/file/CommonTests.cpp

@@ -181,6 +181,31 @@ TEST_F(Functionality, WriteInvalidModes) {
     write_rule.apply(*tester);
 }
 
+// Ensure a write followed by a read produces valid data
+TEST_F(FunctionalIO, WriteSeekAppendReadBack) {
+    Os::Test::File::Tester::OpenFileCreate open_rule(false);
+    Os::Test::File::Tester::OpenForAppend open_append_rule(false);
+    Os::Test::File::Tester::Write write_rule;
+    Os::Test::File::Tester::Seek seek_rule;
+    Os::Test::File::Tester::CloseFile close_rule;
+    Os::Test::File::Tester::OpenForRead open_read;
+    Os::Test::File::Tester::Read read_rule;
+
+    open_rule.apply(*tester);
+    write_rule.apply(*tester);
+    close_rule.apply(*tester);
+    open_read.apply(*tester);
+    close_rule.apply(*tester);
+    open_append_rule.apply(*tester);
+    write_rule.apply(*tester);
+    seek_rule.apply(*tester);
+    write_rule.apply(*tester);
+    close_rule.apply(*tester);
+    open_read.apply(*tester);
+    read_rule.apply(*tester);
+    close_rule.apply(*tester);
+}
+
 // Ensure a write followed by a read produces valid data
 TEST_F(FunctionalIO, WriteReadBack) {
     Os::Test::File::Tester::OpenFileCreate open_rule(false);

Os/test/ut/file/FileRules.cpp

@@ -171,7 +171,7 @@ void Os::Test::File::Tester::assert_file_opened(const std::string &path, Os::Fil
     // When the open mode has been specified assert that is in an exact state
     if (not path.empty() && Os::File::Mode::OPEN_NO_MODE != newly_opened_mode) {
         // Assert file pointer always at beginning when functional
-        if (functional() ) {
+        if (functional()) {
             FwSizeType file_position = std::numeric_limits<FwSizeType>::max();
             ASSERT_EQ(this->m_file.position(file_position), Os::File::Status::OP_OK);
             ASSERT_EQ(file_position, 0);
@@ -258,7 +258,7 @@ bool Os::Test::File::Tester::OpenBaseRule::precondition(const Os::Test::File::Te
 
 void Os::Test::File::Tester::OpenBaseRule::action(Os::Test::File::Tester &state  //!< The test state
 ) {
-    printf("--> Rule: %s \n", this->getName());
+    printf("--> Rule: %s mode %d\n", this->getName(), this->m_mode);
     // Initial variables used for this test
     std::shared_ptr<const std::string> filename = state.get_filename(this->m_random);
 
@@ -322,6 +322,25 @@ Os::Test::File::Tester::OpenForWrite::OpenForWrite(const bool randomize_filename
 
 }
 
+// ------------------------------------------------------------------------------------------------------
+// Rule:  OpenForAppend
+//
+// ------------------------------------------------------------------------------------------------------
+
+Os::Test::File::Tester::OpenForAppend::OpenForAppend(const bool randomize_filename)
+        : Os::Test::File::Tester::OpenBaseRule("OpenForAppend",
+        // Randomized write mode
+                                               Os::File::Mode::OPEN_APPEND,
+        // Randomized overwrite
+                                               false,
+                                               randomize_filename) {
+    // Ensures that a random write mode will work correctly
+    static_assert((Os::File::Mode::OPEN_SYNC_WRITE - 1) == Os::File::Mode::OPEN_WRITE, "Write modes not contiguous");
+    static_assert((Os::File::Mode::OPEN_APPEND - 1) == Os::File::Mode::OPEN_SYNC_WRITE,
+                  "Write modes not contiguous");
+
+}
+
 // ------------------------------------------------------------------------------------------------------
 // Rule:  OpenForRead
 //
@@ -419,7 +438,12 @@ void Os::Test::File::Tester::Write::action(
     printf("--> Rule: %s \n", this->getName());
     U8 buffer[FILE_DATA_MAXIMUM];
     state.assert_file_consistent();
-    FwSizeType size_desired = static_cast<FwSizeType>(STest::Pick::lowerUpper(0, FILE_DATA_MAXIMUM));
+    FwSizeType current_position = 0;
+    state.m_file.position(current_position);
+    if(state.m_mode == Os::File::Mode::OPEN_APPEND) {
+        state.m_file.size(current_position);
+    }
+    FwSizeType size_desired = static_cast<FwSizeType>(STest::Pick::lowerUpper(0, FILE_DATA_MAXIMUM - current_position));
     FwSizeType size_written = size_desired;
     bool wait = static_cast<bool>(STest::Pick::lowerUpper(0, 1));
     for (FwSizeType i = 0; i < size_desired; i++) {
@@ -435,7 +459,7 @@ void Os::Test::File::Tester::Write::action(
 }
 
 // ------------------------------------------------------------------------------------------------------
-// Rule:  Read
+// Rule:  Seek
 //
 // ------------------------------------------------------------------------------------------------------
 
@@ -464,7 +488,8 @@ void Os::Test::File::Tester::Seek::action(
     if (absolute) {
         seek_offset = STest::Pick::lowerUpper(0, FILE_DATA_MAXIMUM);
     } else {
-        seek_offset = STest::Pick::lowerUpper(0, original_file_state.position + FILE_DATA_MAXIMUM) - original_file_state.position;
+        seek_offset = STest::Pick::lowerUpper(0, FILE_DATA_MAXIMUM);
+        seek_offset -= original_file_state.position;
     }
     Os::File::Status status = state.m_file.seek(seek_offset, absolute ? Os::File::SeekType::ABSOLUTE : Os::File::SeekType::RELATIVE);
     ASSERT_EQ(status, Os::File::Status::OP_OK);
@@ -496,8 +521,8 @@ void Os::Test::File::Tester::Preallocate::action(
     printf("--> Rule: %s \n", this->getName());
     state.assert_file_consistent();
     FileState original_file_state = state.current_file_state();
-    FwSizeType offset = static_cast<FwSizeType>(STest::Pick::lowerUpper(0, FILE_DATA_MAXIMUM));
-    FwSizeType length = static_cast<FwSizeType>(STest::Pick::lowerUpper(1, FILE_DATA_MAXIMUM));
+    FwSizeType offset = static_cast<FwSizeType>(STest::Pick::lowerUpper(0, FILE_DATA_MAXIMUM - 1));
+    FwSizeType length = static_cast<FwSizeType>(STest::Pick::lowerUpper(1, FILE_DATA_MAXIMUM - offset));
     Os::File::Status status = state.m_file.preallocate(offset, length);
     ASSERT_EQ(Os::File::Status::OP_OK, status);
     state.shadow_preallocate(offset, length);

Os/test/ut/file/FileRules.hpp

@@ -58,6 +58,19 @@ struct OpenFileCreateOverwrite : public OpenBaseRule {
 };
 
 
+// ------------------------------------------------------------------------------------------------------
+// Rule:  OpenForAppend
+//
+// ------------------------------------------------------------------------------------------------------
+struct OpenForAppend : public OpenBaseRule {
+    // ----------------------------------------------------------------------
+    // Construction
+    // ----------------------------------------------------------------------
+
+    //! Constructor
+    explicit OpenForAppend(const bool randomize_filename = false);
+};
+
 // ------------------------------------------------------------------------------------------------------
 // Rule:  OpenForWrite
 //

Os/test/ut/file/SyntheticFileSystem.cpp

@@ -8,6 +8,10 @@
 namespace Os {
 namespace Test {
 
+SyntheticFile::~SyntheticFile() {
+    this->close();
+}
+
 SyntheticFileSystem::OpenData SyntheticFileSystem::open(const CHAR* char_path, const Os::File::Mode open_mode, const File::OverwriteType overwrite) {
     SyntheticFileSystem::OpenData return_value;
     std::string path = char_path;
@@ -36,7 +40,9 @@ SyntheticFileSystem::OpenData SyntheticFileSystem::open(const CHAR* char_path, c
         if (truncate) {
             return_value.file->m_data.clear();
         }
+
         return_value.file->m_pointer = 0;
+
         return_value.file->m_mode = open_mode;
         return_value.file->m_path = path;
         // Checks on the shadow data to ensure consistency
@@ -83,9 +89,11 @@ void SyntheticFile::close() {
     if (this->m_data != nullptr) {
         this->m_data->m_mode = Os::File::Mode::OPEN_NO_MODE;
         this->m_data->m_path.clear();
+        this->m_data->m_pointer = 0;
         // Checks on the shadow data to ensure consistency
         FW_ASSERT(this->m_data->m_mode == Os::File::Mode::OPEN_NO_MODE);
         FW_ASSERT(this->m_data->m_path.empty());
+        FW_ASSERT(this->m_data->m_pointer == 0);
     }
 }
 

Os/test/ut/file/SyntheticFileSystem.hpp

@@ -36,6 +36,9 @@ class SyntheticFile : public FileInterface {
   public:
     friend class SyntheticFileSystem;
 
+    //! \brief Destructor in order to correctly close file
+    virtual ~SyntheticFile();
+
     //! \brief check if file exists
     static bool exists(const CHAR* path);
 
@@ -59,7 +62,7 @@ class SyntheticFile : public FileInterface {
 
     //! \brief close the file
     //!
-    void close() override;
+    void close() final;
 
     //! \brief read data from the file
     //!

Os/test/ut/filesystem/FileSystemRules.cpp

@@ -54,6 +54,7 @@ bool Os::Test::FileSystem::Tester::DirectoryExists::precondition(const Os::Test:
 
 void Os::Test::FileSystem::Tester::DirectoryExists::action(Os::Test::FileSystem::Tester &state) {
     std::string dirPath = state.get_random_directory().path;
+    printf("--> Rule: %s directory path %s\n", this->getName(), dirPath.c_str());
     ASSERT_TRUE(Os::FileSystem::getSingleton().exists(dirPath.c_str())) << "exists() failed for directory";
 }
 

Svc/FpySequencer/test/ut/FpySequencerTester.cpp

@@ -108,8 +108,9 @@ void FpySequencerTester::addStmt(const Fpy::Statement& stmt) {
     // if fails, cannot add a new stmt (out of space)
     FW_ASSERT(seq.getheader().getstatementCount() < std::numeric_limits<U16>::max());
 
-    seq.getstatements()[seq.getheader().getstatementCount()] = stmt;
-    seq.getheader().setstatementCount(seq.getheader().getstatementCount() + 1);
+    U16 stateCount = seq.getheader().getstatementCount();
+    seq.getstatements()[stateCount] = stmt;
+    seq.getheader().setstatementCount(static_cast<U16>(stateCount + 1));
 }
 
 void FpySequencerTester::addCmd(FwOpcodeType opcode) {