functional-tests: fix code review comments

Signed-off-by: Patryk Wrobel <183546751+pwrobeldev@users.noreply.github.com>

Author: pwrobeldev

functional-tests/functional/android-kotlin/src/test/kotlin/com/here/android/test/AsyncDecoratorTest.kt

@@ -23,6 +23,8 @@ import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.runner.RunWith
+import org.junit.After
+import org.junit.Before
 import org.junit.Test
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.annotation.Config
@@ -34,11 +36,23 @@ data class SuspendingJobResult(val labels: List<String>, val timestamp: Long, va
 @RunWith(RobolectricTestRunner::class)
 @Config(application = RobolectricApplication::class)
 class AsyncDecoratorTest {
-    data class SuspendingJobResult(val labels: List<String>, val timestamp: Long)
+    var testedEngine: CoolEngine? = null
+
+    @Before
+    fun setUp() {
+        // Create the engine used in tests.
+        testedEngine = CoolEngine()
+    }
+
+    @After
+    fun tearDown() {
+        // Ensure that underlying C++ threads join.
+        testedEngine?.waitForCompletion()
+    }
 
     @Test
     fun multipleSuspendingFunctionsExecutedConcurrently() {
-        val engine = CoolEngine()
+        val engine = testedEngine!!
         val startTimestamp = System.currentTimeMillis()
 
         // Test scenario is as follows:
@@ -56,12 +70,12 @@ class AsyncDecoratorTest {
         //  - this way we can tell that they were executed concurrently and there was no blocking -- if there was blocking
         //    then elapsed time would be total of given task and the ones before it
         runBlocking {
-            // This task should work for a bit longer than 2000 ms.
+            // This task should work for a bit longer than 200 ms.
             val t0 = async {
-                // Worker thread on C++ level will sleep for: 1000 ms.
+                // Worker thread on C++ level will sleep for: 100 ms.
                 val coolLabels: List<String> = engine.downloadCoolLabelsAsync("cool-labels.com")
 
-                // Worker thread on C++ level will sleep for: 1000 ms.
+                // Worker thread on C++ level will sleep for: 100 ms.
                 val superLabels: List<String> = engine.downloadCoolLabelsAsync("my-super-labels.com")
 
                 // Return all labels from the task + timestamp.
@@ -72,15 +86,15 @@ class AsyncDecoratorTest {
                 )
             }
 
-            // This task should work for a bit longer than 6000ms.
+            // This task should work for a bit longer than 600ms.
             val t1 = async {
-                // Worker thread on C++ level will sleep for: 1000 ms.
+                // Worker thread on C++ level will sleep for: 100 ms.
                 val coolLabels: List<String> = engine.downloadCoolLabelsAsync("cool-labels.com")
 
-                // Introduce artificial delay of 2000ms.
-                delay(2000)
+                // Introduce artificial delay of 200ms.
+                delay(200)
 
-                // Worker thread on C++ level will sleep for: 3000 ms.
+                // Worker thread on C++ level will sleep for: 300 ms.
                 val dummyLabels: List<String> = engine.downloadCoolLabelsAsync("dummy-labels.com")
 
                 // Return all labels from the task + timestamp.
@@ -91,13 +105,13 @@ class AsyncDecoratorTest {
                 )
             }
 
-            // This task should finish with exception after a bit longer than 500ms.
+            // This task should finish with exception after a bit longer than 50ms.
             val t2 = async {
                 var exceptionCaught: Boolean = false
                 var labels = emptyList<String>()
 
                 try {
-                    // This url is not handled. It will throw after 500ms.
+                    // This url is not handled. It will throw after 50ms.
                     labels = engine.downloadCoolLabelsAsync("this-url-will-throw.com")
                 } catch (e: Exception) {
                     exceptionCaught = true
@@ -116,33 +130,30 @@ class AsyncDecoratorTest {
             // Check that expected labels were obtained without exception.
             assertEquals(listOf("COOL_LABEL", "SUPER_LABEL", "ANOTHER_SUPER_LABEL"), t0Result.labels)
             assertFalse(t0Result.exceptionCaught)
-            // Check that job took at least 2000ms, and less than 2500ms (margin).
+            // Check that job took at least 200ms, and less than 250ms (margin).
             val t0ElapsedTimeMs = t0Result.timestamp - startTimestamp
-            assertTrue("Time of the first job: $t0ElapsedTimeMs >= 2000ms", t0ElapsedTimeMs >= 2000)
-            assertTrue("Time of the first job: $t0ElapsedTimeMs < 2500ms", t0ElapsedTimeMs < 2500)
+            assertTrue("Time of the first job: $t0ElapsedTimeMs >= 200ms", t0ElapsedTimeMs >= 200)
+            assertTrue("Time of the first job: $t0ElapsedTimeMs < 250ms", t0ElapsedTimeMs < 250)
 
             // Wait for the second task's result.
             val t1Result = t1.await()
             // Check that expected labels were obtained without exception.
             assertEquals(listOf("COOL_LABEL", "DUMMY_LABEL"), t1Result.labels)
             assertFalse(t1Result.exceptionCaught)
-            // Check that job took at least 6000ms, and less than 6500ms (margin).
+            // Check that job took at least 600ms, and less than 650ms (margin).
             val t1ElapsedTimeMs = t1Result.timestamp - startTimestamp
-            assertTrue("Time of the second job: $t1ElapsedTimeMs >= 6000ms", t1ElapsedTimeMs >= 6000)
-            assertTrue("Time of the second job: $t1ElapsedTimeMs < 6500ms", t1ElapsedTimeMs < 6500)
+            assertTrue("Time of the second job: $t1ElapsedTimeMs >= 600ms", t1ElapsedTimeMs >= 600)
+            assertTrue("Time of the second job: $t1ElapsedTimeMs < 650ms", t1ElapsedTimeMs < 650)
 
             // Wait for the third task's result.
             val t2Result = t2.await()
             // Check that expected labels were obtained without exception.
             assertTrue(t2Result.labels.isEmpty())
             assertTrue(t2Result.exceptionCaught)
-            // Check that job took at least 500ms, and less than 1000ms (margin).
+            // Check that job took at least 50ms, and less than 100ms (margin).
             val t2ElapsedTimeMs = t2Result.timestamp - startTimestamp
-            assertTrue("Time of the second job: $t2ElapsedTimeMs >= 500ms", t2ElapsedTimeMs >= 500)
-            assertTrue("Time of the second job: $t2ElapsedTimeMs < 1000ms", t2ElapsedTimeMs < 1000)
+            assertTrue("Time of the second job: $t2ElapsedTimeMs >= 500ms", t2ElapsedTimeMs >= 50)
+            assertTrue("Time of the second job: $t2ElapsedTimeMs < 1000ms", t2ElapsedTimeMs < 100)
         }
-
-        // Ensure that underlying C++ threads join.
-        engine.waitForCompletion()
     }
 }

functional-tests/functional/input/src/cpp/AsyncDecorator.cpp

@@ -45,23 +45,23 @@ download_cool_labels(const std::string& url)
 
     if (url == "my-super-labels.com")
     {
-        std::this_thread::sleep_for(1000ms);
+        std::this_thread::sleep_for(100ms);
         return std::vector<std::string>{"SUPER_LABEL", "ANOTHER_SUPER_LABEL"};
     }
 
     if (url == "cool-labels.com")
     {
-        std::this_thread::sleep_for(1000ms);
+        std::this_thread::sleep_for(100ms);
         return std::vector<std::string>{"COOL_LABEL"};
     }
 
     if (url == "dummy-labels.com")
     {
-        std::this_thread::sleep_for(3000ms);
+        std::this_thread::sleep_for(300ms);
         return std::vector<std::string>{"DUMMY_LABEL"};
     }
 
-    std::this_thread::sleep_for(500ms);
+    std::this_thread::sleep_for(50ms);
     return std::nullopt;
 }
 
@@ -74,12 +74,11 @@ class NonCancellableAsyncTask : public AsyncTaskHandle
         {
             m_worker = std::thread{
                 [this, w = std::move(do_work)] {
+                    m_is_running.store(true);
                     w();
                     m_is_running.store(false);
                 }
             };
-
-            m_is_running.store(true);
         }
     }
 
@@ -128,15 +127,15 @@ class CoolEngineImpl : public CoolEngine
     std::shared_ptr<test::AsyncTaskHandle>
     download_cool_labels_async(const ::std::string& url, const ::test::EngineWorkCompletedCallback& callback) override
     {
-        std::function<void()> do_work = [url, c = std::move(callback)] {
+        std::function<void()> do_work = [url, callback] {
             auto labels = download_cool_labels(url);
             if (labels.has_value())
             {
-                c({}, *labels);
+                callback({}, *labels);
             }
             else
             {
-                c(EngineError::ENGINE_ON_FIRE, std::nullopt);
+                callback(EngineError::ENGINE_ON_FIRE, std::nullopt);
             }
         };