From 676bacb77c70dc692e91d17702f22314009eba94 Mon Sep 17 00:00:00 2001
From: Aaron Colwell <300262+acolwell@users.noreply.github.com>
Date: Tue, 17 Oct 2023 12:51:52 -0700
Subject: [PATCH] Fix asserts and build buster in debug build.

- Fixed assert() in Tests/FileSystemModel_Test.cpp that broke the
  debug build.
- Fixed asserts in Engine/EffectInstanceRenderRoI.cpp so that they
  are only checked if tiling is supported. The assert was
  accidentally applied to all cases when the code was refactored
  by https://github.com/NatronGitHub/Natron/pull/918
- Updated Linux CI to build and test release AND debug builds.
  This should help avoid such issues from sneaking in going forward.
---
 .github/workflows/ci.yml           | 39 +++++++++++++++++++++---------
 Engine/EffectInstanceRenderRoI.cpp | 16 +++++++-----
 Tests/FileSystemModel_Test.cpp     |  8 +++---
 3 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 41bc38f7f..4c6f88a4e 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -66,34 +66,49 @@ jobs:
           tar xzf Natron-v${OCIO_CONFIG_VERSION}.tar.gz
           mv OpenColorIO-Configs-Natron-v${OCIO_CONFIG_VERSION} OpenColorIO-Configs
 
-      - name: Build Unix
+      - name: Download Plugins
         run: |
-          mkdir build && cd build
+          mkdir Plugins && cd Plugins
+          wget https://github.com/NatronGitHub/openfx-io/releases/download/natron_testing/openfx-io-build-ubuntu_22-testing.zip
+          unzip openfx-io-build-ubuntu_22-testing.zip
+          cd ..
+
+      - name: Build Unix (debug)
+        run: |
+          mkdir debug && cd debug
           cmake ../
           make -j2
 
-      - name: Run Unix Tests
-        id: run-unix-tests
+      - name: Run Unix Tests (debug)
+        id: run-unix-tests-debug
         # Allow continuing after error so logs can be uploaded.
         continue-on-error: true
         run: |
-          cd build
+          cd debug
+          OFX_PLUGIN_PATH=$PWD/../Plugins OCIO=$PWD/../OpenColorIO-Configs/blender/config.ocio ctest -V
 
-          mkdir Plugins && cd Plugins
-          wget https://github.com/NatronGitHub/openfx-io/releases/download/natron_testing/openfx-io-build-ubuntu_22-testing.zip
-          unzip openfx-io-build-ubuntu_22-testing.zip
-          cd ..
+      - name: Build Unix (release)
+        run: |
+          mkdir release && cd release
+          cmake ../
+          make -j2
 
-          OFX_PLUGIN_PATH=$PWD/Plugins OCIO=$PWD/../OpenColorIO-Configs/blender/config.ocio ctest -V
+      - name: Run Unix Tests (release)
+        id: run-unix-tests-release
+        # Allow continuing after error so logs can be uploaded.
+        continue-on-error: true
+        run: |
+          cd release
+          OFX_PLUGIN_PATH=$PWD/../Plugins OCIO=$PWD/../OpenColorIO-Configs/blender/config.ocio ctest -V
 
       - name: Upload ${{ matrix.os }} Test Log artifacts
         uses: actions/upload-artifact@v3
         with:
           name: ${{ matrix.os }} Test Logs
-          path: ${{ github.workspace }}/build/Testing/Temporary/LastTest.log
+          path: ${{ github.workspace }}/release/Testing/Temporary/LastTest.log
 
       - name: Check for test failures
-        if: steps.run-unix-tests.outcome == 'failure'
+        if: steps.run-unix-tests-debug.outcome == 'failure' || steps.run-unix-tests-release.outcome == 'failure'
         run: exit 1
 
   win-test:
diff --git a/Engine/EffectInstanceRenderRoI.cpp b/Engine/EffectInstanceRenderRoI.cpp
index a9a614135..dcaa11088 100644
--- a/Engine/EffectInstanceRenderRoI.cpp
+++ b/Engine/EffectInstanceRenderRoI.cpp
@@ -737,17 +737,21 @@ EffectInstance::renderRoI(const RenderRoIArgs & args,
         //renderRoIInternal should check the bitmap of 'image' and not downscaledImage!
         roi = args.roi.toNewMipmapLevel(args.mipmapLevel, 0, par, rod);
 
-        if (frameArgs->tilesSupported && !roi.clipIfOverlaps(upscaledImageBoundsNc)) {
-            return eRenderRoIRetCodeOk;
+        if (frameArgs->tilesSupported) {
+            if (!roi.clipIfOverlaps(upscaledImageBoundsNc)) {
+                return eRenderRoIRetCodeOk;
+            }
+            assert(upscaledImageBoundsNc.contains(roi));
         }
-        assert( upscaledImageBoundsNc.contains(roi));
     } else {
         roi = args.roi;
 
-        if (frameArgs->tilesSupported && !roi.clipIfOverlaps(downscaledImageBoundsNc)) {
-            return eRenderRoIRetCodeOk;
+        if (frameArgs->tilesSupported) {
+            if (!roi.clipIfOverlaps(downscaledImageBoundsNc)) {
+                return eRenderRoIRetCodeOk;
+            }
+            assert(downscaledImageBoundsNc.contains(roi));
         }
-        assert(downscaledImageBoundsNc.contains(roi));
     }
 
     /*
diff --git a/Tests/FileSystemModel_Test.cpp b/Tests/FileSystemModel_Test.cpp
index 9ea83ab40..9d7e09b3f 100644
--- a/Tests/FileSystemModel_Test.cpp
+++ b/Tests/FileSystemModel_Test.cpp
@@ -249,10 +249,8 @@ TEST(FileSystemModelTest, CleanPath) {
       expectedOutput = testCase.input;
     }
 #endif
-    // Make sure the expectation was actually set to something.
-    assert(!expectedOutput.isNull());
-
-    const auto output = FileSystemModel::cleanPath(input).toStdString();
-    EXPECT_EQ(expectedOutput, output) << " input '" << testCase.input << "'";
+    const QString output = FileSystemModel::cleanPath(input);
+    ASSERT_TRUE(!output.isNull());
+    EXPECT_EQ(expectedOutput, output.toStdString()) << " input '" << testCase.input << "'";
   }
 }
\ No newline at end of file