- Debug parser (carriage returns)

- Improve test framework

.gitlab-ci.yml

@@ -3,4 +3,4 @@ archlinux:
   before_script:
     - pacman -Sy base-devel cmake --noconfirm --needed
   script:
-    - mkdir build && cd build && cmake ../ && make && cd tests && ./pgnp_tests
+    - mkdir build && cd build && cmake ../ && make && ctest

CMakeLists.txt

@@ -15,4 +15,5 @@ configure_file(src/LargeFileStream.hpp ${PGNP_INCLUDE_DIR} COPYONLY)
 include_directories(${PGNP_INCLUDE_DIR})
 
 # Unit tests
+enable_testing()
 add_subdirectory(./tests)

src/LargeFileStream.cpp

@@ -58,6 +58,6 @@ char LargeFileStream::operator[](long loc) {
   return buffer[offset];
 }
 
-bool LargeFileStream::IsEOF(long loc) { return (eof); }
+bool LargeFileStream::IsEOF() { return (eof); }
 
 } // namespace pgnp

src/LargeFileStream.hpp

@@ -36,7 +36,7 @@ public:
   /// @brief Allow array like access to the file
   char operator[](long loc);
   /// @brief Check if we reach the EOF
-  bool IsEOF(long loc);
+  bool IsEOF();
 
   // Various Exceptions
   struct BackwardRead : public std::exception {

src/PGN.cpp

@@ -3,14 +3,14 @@
 #include <iostream>
 #include <string>
 
-#define IS_BLANK(c) (c == ' ' || c == '\n' || c == '\t')
+#define IS_BLANK(c) (c == ' ' || c == '\n' || c == '\t' || c == '\r')
 #define IS_DIGIT(c)                                                            \
   (c == '0' || c == '1' || c == '2' || c == '3' || c == '4' || c == '5' ||     \
    c == '6' || c == '7' || c == '8' || c == '9')
-#define IS_EOF(loc) (pgn_content.IsEOF(loc))
+#define IS_EOF (pgn_content.IsEOF())
 #define EOF_CHECK(loc)                                                         \
   {                                                                            \
-    if (IS_EOF(loc))                                                           \
+    if (IS_EOF)                                                           \
       throw UnexpectedEOF();                                                   \
   }
 
@@ -42,17 +42,16 @@ void PGN::ParseNextGame() {
   moves = new HalfMove();
 
   // Search for new game
-  if (IS_EOF(LastGameEndLoc)) {
+  if (IS_EOF) {
     throw NoGameFound();
   }
   int loc = NextNonBlank(LastGameEndLoc);
-
-  if (IS_EOF(loc)) {
+  if (IS_EOF) {
     throw NoGameFound();
   }
 
   // Parse game
-  while (!IS_EOF(loc)) {
+  while (!IS_EOF) {
     char c = pgn_content[loc];
     if (!IS_BLANK(c)) {
       if (c == '[') {
@@ -108,7 +107,8 @@ long PGN::ParseComment(long loc, HalfMove *hm) {
   EOF_CHECK(loc);
   char c = pgn_content[loc];
 
-  if (c == '{') {
+  // Parse a sequence of comment
+  while (!IS_EOF && c == '{') {
     loc++;
     EOF_CHECK(loc);
     c = pgn_content[loc];
@@ -119,6 +119,12 @@ long PGN::ParseComment(long loc, HalfMove *hm) {
       c = pgn_content[loc];
     }
     loc++; // Skip '}'
+
+    // Goto next non blank to look for another comment token
+    loc = NextNonBlank(loc);
+    if(!IS_EOF){
+      c = pgn_content[loc];
+    }
   }
   return (loc);
 }
@@ -204,7 +210,7 @@ long PGN::ParseHalfMove(long loc, HalfMove *hm) {
 
   // Check for variations
   loc = NextNonBlank(loc);
-  while (!IS_EOF(loc) && pgn_content[loc] == '(') {
+  while (!IS_EOF && pgn_content[loc] == '(') {
     loc++; // Skip '('
     HalfMove *var = new HalfMove;
     loc = ParseHalfMove(loc, var);
@@ -226,7 +232,7 @@ long PGN::ParseHalfMove(long loc, HalfMove *hm) {
 
   // Parse next HalfMove
   loc = NextNonBlank(loc);
-  if (!IS_EOF(loc)) {
+  if (!IS_EOF) {
     HalfMove *next_hm = new HalfMove;
     next_hm->count = hm->count;
     loc = ParseHalfMove(loc, next_hm);
@@ -259,7 +265,7 @@ long PGN::ParseNextTag(long start_loc) {
   long valueloc = NextNonBlank(keyloc) + 1;
   EOF_CHECK(keyloc);
   c = pgn_content[valueloc];
-  while (c != '"' or IS_EOF(valueloc)) {
+  while (c != '"' or IS_EOF) {
     value += c;
     valueloc++;
     EOF_CHECK(keyloc);
@@ -308,7 +314,7 @@ long PGN::NextNonBlank(long loc) {
   char c = pgn_content[loc];
   while (IS_BLANK(c)) {
     loc++;
-    if (IS_EOF(loc)) {
+    if (IS_EOF) {
       return (loc);
     }
     c = pgn_content[loc];

src/PGN.hpp

@@ -58,6 +58,7 @@ private:
   long NextNonBlank(long);
   /// @brief Parse a HalfMove at a specific location into @a pgn_content
   long ParseHalfMove(long, HalfMove *);
+  /// @brief Parse a consecutive sequence of comment
   long ParseComment(long, HalfMove *);
 };
 

tests/CMakeLists.txt

@@ -1,8 +1,20 @@
-include_directories(./catch3/)
 
 # Copy asset files
 file(COPY pgn_files DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/)
 
-# Run tests
-add_executable(pgnp_tests tests.cpp ./catch3/catch_amalgamated.cpp)
-target_link_libraries(pgnp_tests pgnp)
+# Configure catch3
+include_directories(./catch3/)
+add_library(catch3 SHARED ./catch3/catch_amalgamated.cpp)
+
+# Add tests
+add_executable(pgnp_valid valid.cpp)
+target_link_libraries(pgnp_valid pgnp catch3)
+add_test(PGNP_Valid_PGN_Set pgnp_valid)
+
+add_executable(pgnp_str str.cpp)
+target_link_libraries(pgnp_str pgnp catch3)
+add_test(PGNP_STR_Compliant_Set pgnp_str)
+
+add_executable(pgnp_combined combined.cpp)
+target_link_libraries(pgnp_combined pgnp catch3)
+add_test(PGNP_Combined_Set pgnp_combined)

tests/combined.cpp

@@ -0,0 +1,43 @@
+#include "pgnp.hpp"
+#include <catch_amalgamated.hpp>
+
+using namespace pgnp;
+
+TEST_CASE("Hartwig PGN", "[combined/hartwig]") {
+  // PGN source: https://www.angelfire.com/games3/smartbridge/
+
+  pgnp::PGN pgn;
+  pgn.FromFile("pgn_files/combined/hartwig.pgn");
+
+  // Count games
+  REQUIRE_NOTHROW([&]() {
+    char i = 0;
+    try {
+      while (true) {
+        pgn.ParseNextGame();
+        i++;
+      }
+    } catch (const NoGameFound &e) {
+      CHECK(i == 29);
+    }
+  }());
+
+  SECTION("Check comments of a game") {
+    pgnp::PGN pgn;
+    pgn.FromFile("pgn_files/combined/hartwig.pgn");
+    pgn.ParseNextGame();
+    pgn.ParseNextGame();
+    pgn.ParseNextGame();
+    pgn.ParseNextGame();
+    pgn.ParseNextGame(); // Load game 5
+
+    HalfMove *m = new HalfMove();
+    pgn.GetMoves(m);
+    std::cout << m->comment;
+    CHECK(m->comment ==
+          "I had actually prepared 1.d4 for the tournament, but I backed out "
+          "in every (!) game for various different reasons. In this case it "
+          "was because things were in such a rut I would only be cheered by "
+          "winning in crushing style.  Thankfully it worked!");
+  }
+}

tests/str.cpp

@@ -0,0 +1,17 @@
+#include "pgnp.hpp"
+#include <catch_amalgamated.hpp>
+
+using namespace pgnp;
+
+TEST_CASE("Seven Tag Roster", "[std/pgn1]") {
+  PGN pgn;
+  REQUIRE_NOTHROW(pgn.FromFile("pgn_files/str/pgn1.pgn"));
+  REQUIRE_NOTHROW(pgn.ParseNextGame());
+
+  REQUIRE_NOTHROW(pgn.STRCheck());
+  HalfMove *m = new HalfMove();
+  pgn.GetMoves(m);
+  REQUIRE(m->GetLength() == 85);
+  CHECK(pgn.GetResult() == "1/2-1/2");
+  REQUIRE_THROWS_AS(pgn.ParseNextGame(),NoGameFound);
+}

tests/valid.cpp

@@ -87,16 +87,3 @@ TEST_CASE("Valid PGN", "[valid/pgn2]") {
   }
   REQUIRE_THROWS_AS(pgn.ParseNextGame(),NoGameFound);
 }
-
-TEST_CASE("Seven Tag Roster", "[std/pgn1]") {
-  PGN pgn;
-  REQUIRE_NOTHROW(pgn.FromFile("pgn_files/str/pgn1.pgn"));
-  REQUIRE_NOTHROW(pgn.ParseNextGame());
-
-  REQUIRE_NOTHROW(pgn.STRCheck());
-  HalfMove *m = new HalfMove();
-  pgn.GetMoves(m);
-  REQUIRE(m->GetLength() == 85);
-  CHECK(pgn.GetResult() == "1/2-1/2");
-  REQUIRE_THROWS_AS(pgn.ParseNextGame(),NoGameFound);
-}