Merge pull request #1771 from obsidian-tasks-group/mock_tests-for-tog…

…gling

test: Create mock tests for File.replaceTaskWithTasks()

resources/sample_vaults/Tasks-Demo/Manual Testing/Task Toggling Scenarios/1680 - Reading Mode line numbers not updated on editing.md

@@ -2,32 +2,91 @@
 
 ## Instructions
 
-**Given** a file is open in both two panes:
+> [!Tip] Goal
+> **Given** a file is open in both two panes:
+>
+> - an editing mode (Source or Live Preview)
+> - Reading mode
+>
+> **When** I add or remove lines from the file
+>
+> **Then** rendered tasks in the Reading pane should be updated, so that when I toggle them, the correct line is toggled.
 
-- an editing mode (Source or Live Preview)
-- Reading mode
+## Testing steps
 
-**When** I add or remove lines from the file
+> [!Todo] Testing Steps
+>
+> - 1 **Setup**
+>   - Open this file in Source View
+>   - Open it in a second pane, in Reading View
+> - 2 **Add some white space before the tasks**
+>   - After the line containing `In step 2`, enter 5 blank lines
+> - 3 **Toggle a task in each section, in the Reading View pane**
+>   - Toggle `Section 1/Task 1` to Done then to TODO
+>   - Toggle `Section 2/Task 2` to Done then to TODO
+> - 4 **Quickly delete the blank lines then toggle `Section 2/Task 1`**
+>   - Select the 5 added lines
+>   - Move the cursor next to the checkbox of `Section 2/Task 1`
+>   - Hit the delete key to delete the blank lines
+>   - **As quickly as possible** (and in under 2 seconds) **toggle `Section 2/Task 1`**
+>   - **As quickly as possible** (and in under 2 seconds) **toggle `Section 2/Task 2`**
 
-**Then** rendered tasks in the Reading pane should be updated, so that when I toggle them, the correct line is toggled.
+### Expected result
 
-## Tasks Section 1
+> [!Success]
+>
+> - `Section 2/Task 1` is marked as Done
+> - `Section 2/Task 2` is marked as Done
 
-Some random tasks
+---
+
+## Test Data
+
+### Tasks Section 1
+
+In step 2, Insert 5 blank lines after this line.
+
+Some random tasks:
 
 - [ ] #task Section 1/Task 1
 - [ ] #task Section 1/Task 2
 
-## Tasks Section 2
+### Tasks Section 2
 
-Some more random tasks
+Some more random tasks:
 
 - [ ] #task Section 2/Task 1
 - [ ] #task Section 2/Task 2
 
-## Query results
+---
+
+## Log of actual results - newest first
+
+### Actual result in `5649417`, PR #1702, 2023-02-27
+
+<https://github.com/obsidian-tasks-group/obsidian-tasks/commit/5649417006a5ccf80cdaaaeb2bc404db5c6f6a86>
+
+> [!Bug]
+>
+> - Two console error messages:
+>
+> ```text
+> plugin:obsidian-tasks-plugin:17532 Tasks: could not find task to toggle in the file.
+> plugin:obsidian-tasks-plugin:17532 Tasks: could not find task to toggle in the file.```
+> ```
+
+### Actual result in Tasks 1.25.0
 
-```tasks
-path includes 1680 - Reading Mode line numbers not updated on editing
-sort by description
-```
+> [!Bug]
+>
+> - **Step 3**
+>   - Sometimes the task line that was toggled is copied over an earlier task line in the file, losing that original line
+> - **Step 4**
+>   - `Section 2/Task 1` is marked as Done
+>   - Toggling `Section 2/Task 1`:
+>     - sometimes works, but Obsidian shows a notice about the file being modified externally, and changes are being merged
+>     - sometimes fails, giving console message:
+>
+> ```text
+> Tasks: could not find task to toggle in the file.
+> ```

resources/sample_vaults/Tasks-Demo/Manual Testing/Task Toggling Scenarios/Embed Task in to Note.md

@@ -1,32 +1,24 @@
 # Embed Task in to Note
 
-See [When a block-referenced task is made complete, it is completed after rewriting the contents of another task. · Issue #688](https://github.com/obsidian-tasks-group/obsidian-tasks/issues/688)
+## Scenario: Embedding individual tasks
+
+- **Given** a bullet list which embeds a task via a block reference, in the same file or another file
+- **When** the embedded task is toggled
+- **Then** there is an error message saying that the task with the block reference could not be found, to be toggled
+  - See [Issue #688](https://github.com/obsidian-tasks-group/obsidian-tasks/issues/688)
 
 ## Category 1
 
 - [ ] #task task1a
 - [ ] #task task1b
-- [ ] #task task1c
 
 ## Category 2
 
 - [ ] #task task2a
 - [ ] #task task2b ^ca47c7
-- [ ] #task task2c
-
-## Scenario 3: Embedding individual tasks
-
-- **Given** A bullet list which embeds a task via a block reference, in the same file or another file
-- **When** When the embedded task is toggled
-- **Then** There is an error message saying that the task with the block reference could not be found, to be toggled
-  - See [Issue #688](https://github.com/obsidian-tasks-group/obsidian-tasks/issues/688)
 
 ## The embedded tasks
 
----
+To reproduce issue 688, click on the following checkbox in Reading mode:
 
 - ref task2 ![[#^ca47c7]]
-
----
-
-- ref task4 ![[Embed File in to Note - File to Embed#^fromseparatefile]]

src/File.ts

@@ -1,7 +1,7 @@
-import { MarkdownView, MetadataCache, Notice, TFile, Vault, Workspace } from 'obsidian';
-import type { ListItemCache } from 'obsidian';
+import { type ListItemCache, MarkdownView, MetadataCache, Notice, TFile, Vault, Workspace } from 'obsidian';
 
 import { getSettings } from './Config/Settings';
+import { type MockListItemCache, type MockTask, saveMockDataForTesting } from './lib/MockDataCreator';
 import type { Task } from './Task';
 
 let metadataCache: MetadataCache | undefined;
@@ -12,6 +12,8 @@ let workspace: Workspace | undefined;
 const supportedFileExtensions = ['md'];
 const supportedViewTypes = [MarkdownView];
 
+export type ErrorLoggingFunction = (message: string) => void;
+
 export const initializeFile = ({
     metadataCache: newMetadataCache,
     vault: newVault,
@@ -150,11 +152,51 @@ const tryRepetitive = async ({
     const fileContent = await vault.read(file); // TODO: replace with vault.process.
     const fileLines = fileContent.split('\n');
 
+    const taskLineNumber = findLineNumberOfTaskToToggle(originalTask, fileLines, listItemsCache, errorAndNotice);
+
+    if (taskLineNumber === undefined) {
+        const logDataForMocking = false;
+        if (logDataForMocking) {
+            // There was an error finding the correct line to toggle,
+            // so write out to the console a representation of the data needed to reconstruct the above
+            // findLineNumberOfTaskToToggle() call, so that the content can be saved
+            // to a JSON file and then re-used in a 'unit' test.
+            saveMockDataForTesting(originalTask, fileLines, listItemsCache);
+        }
+        errorAndNotice('Tasks: could not find task to toggle in the file.');
+        return;
+    }
+
+    const updatedFileLines = [
+        ...fileLines.slice(0, taskLineNumber),
+        ...newTasks.map((task: Task) => task.toFileLineString()),
+        ...fileLines.slice(taskLineNumber + 1), // Only supports single-line tasks.
+    ];
+
+    await vault.modify(file, updatedFileLines.join('\n'));
+};
+
+/**
+ * Try to find the line number of the originalTask
+ * @param originalTask - the {@link Task} line that the user clicked on
+ * @param fileLines - the lines read from the file.
+ * @param listItemsCache
+ * @param errorLoggingFunction - a function of type {@link ErrorLoggingFunction} - which will be called if the found
+ *                               line differs from the original markdown in {@link originalTask}.
+ *                               This parameter is provided to allow tests to be written for this code
+ *                               that do not display a popup warning, but instead capture the error message.
+ */
+export function findLineNumberOfTaskToToggle(
+    originalTask: Task | MockTask,
+    fileLines: string[],
+    listItemsCache: ListItemCache[] | MockListItemCache[],
+    errorLoggingFunction: ErrorLoggingFunction,
+) {
     const { globalFilter } = getSettings();
-    let listItem: ListItemCache | undefined;
+    let taskLineNumber: number | undefined;
     let sectionIndex = 0;
     for (const listItemCache of listItemsCache) {
-        if (listItemCache.position.start.line < originalTask.sectionStart) {
+        if (listItemCache.position.start.line < originalTask.taskLocation.sectionStart) {
             continue;
         }
 
@@ -164,14 +206,14 @@ const tryRepetitive = async ({
 
         const line = fileLines[listItemCache.position.start.line];
         if (line.includes(globalFilter)) {
-            if (sectionIndex === originalTask.sectionIndex) {
+            if (sectionIndex === originalTask.taskLocation.sectionIndex) {
                 if (line === originalTask.originalMarkdown) {
-                    listItem = listItemCache;
+                    taskLineNumber = listItemCache.position.start.line;
                 } else {
-                    errorAndNotice(
-                        `Tasks: Unable to find task in file ${originalTask.path}.
+                    errorLoggingFunction(
+                        `Tasks: Unable to find task in file ${originalTask.taskLocation.path}.
 Expected task:
-${originalTask.toFileLineString()}
+${originalTask.originalMarkdown}
 Found task:
 ${line}`,
                     );
@@ -183,16 +225,5 @@ ${line}`,
             sectionIndex++;
         }
     }
-    if (listItem === undefined) {
-        errorAndNotice('Tasks: could not find task to toggle in the file.');
-        return;
-    }
-
-    const updatedFileLines = [
-        ...fileLines.slice(0, listItem.position.start.line),
-        ...newTasks.map((task: Task) => task.toFileLineString()),
-        ...fileLines.slice(listItem.position.start.line + 1), // Only supports single-line tasks.
-    ];
-
-    await vault.modify(file, updatedFileLines.join('\n'));
-};
+    return taskLineNumber;
+}

src/lib/MockDataCreator.ts

@@ -0,0 +1,93 @@
+import type { ListItemCache, Pos } from 'obsidian';
+import type { Task } from '../Task';
+
+type MockTaskLocation = {
+    path: string;
+    lineNumber: number;
+    sectionStart: number;
+    sectionIndex: number;
+    precedingHeader: string | null;
+};
+
+/** a mock for {@link ListItemCache.task} */
+type MockListItemCacheTask = string | undefined;
+
+/** a mock for {@link Pos} */
+type MockPos = Pos;
+
+/** a mock for {@link ListItemCache} */
+export type MockListItemCache = { task: string | undefined; position: Pos };
+type MockListItemCaches = MockListItemCache[];
+
+/** a mock for {@link Task} */
+export type MockTask = {
+    originalMarkdown: string;
+    taskLocation: MockTaskLocation;
+};
+
+/** All the data required to call {@link findLineNumberOfTaskToToggle} */
+export type MockTogglingDataForTesting = {
+    cacheData: { listItemsCache: MockListItemCache[] };
+    fileData: { fileLines: string[] };
+    taskData: MockTask;
+};
+
+/**
+ * This function can be used to save data that is used
+ * when finding which line to toggle in a file.
+ * @param originalTask
+ * @param fileLines
+ * @param listItemsCache
+ *
+ * @see saveMockDataForTesting
+ */
+export function getMockDataForTesting(
+    originalTask: Task,
+    fileLines: string[],
+    listItemsCache: ListItemCache[],
+): MockTogglingDataForTesting {
+    const allDataFromListItemCache: MockListItemCaches = [];
+    for (const listItemCache of listItemsCache) {
+        const pos: MockPos = listItemCache.position;
+        const task: MockListItemCacheTask = listItemCache.task;
+        const dataFromListItemCache: MockListItemCache = {
+            position: pos,
+            task: task,
+        };
+        allDataFromListItemCache.push(dataFromListItemCache);
+    }
+    const mockTaskLocation: MockTaskLocation = {
+        path: originalTask.taskLocation.path,
+        lineNumber: originalTask.taskLocation.lineNumber,
+        sectionStart: originalTask.taskLocation.sectionStart,
+        sectionIndex: originalTask.taskLocation.sectionIndex,
+        precedingHeader: originalTask.taskLocation.precedingHeader,
+    };
+    return {
+        taskData: {
+            originalMarkdown: originalTask.originalMarkdown,
+            taskLocation: mockTaskLocation,
+        },
+        fileData: {
+            fileLines: fileLines,
+        },
+        cacheData: {
+            listItemsCache: allDataFromListItemCache,
+        },
+    };
+}
+
+/**
+ * Write the supplied data to the console, so it can be saved for use in testing.
+ *
+ * @param originalTask
+ * @param fileLines
+ * @param listItemsCache
+ */
+export function saveMockDataForTesting(originalTask: Task, fileLines: string[], listItemsCache: ListItemCache[]) {
+    const everything = getMockDataForTesting(originalTask, fileLines, listItemsCache);
+    console.error(`Inconsistent lines: SAVE THE OUTPUT
+data:
+${JSON.stringify(everything)}
+`);
+}

tests/File.test.replaceTaskWithTasks_issue 688 - block referenced task correct behaviour.approved.txt

@@ -0,0 +1,5 @@
+Tasks: Unable to find task in file Manual Testing/Task Toggling Scenarios/Embed Task in to Note.md.
+Expected task:
+- [ ] #task task2b ^ca47c7
+Found task:
+- [ ] #task task1a

tests/File.test.replaceTaskWithTasks_issue 688 - block referenced task current incorrect behaviour.approved.txt

@@ -0,0 +1,5 @@
+Tasks: Unable to find task in file Manual Testing/Task Toggling Scenarios/Embed Task in to Note.md.
+Expected task:
+- [ ] #task task2b ^ca47c7
+Found task:
+- [ ] #task task1a

tests/File.test.replaceTaskWithTasks_valid 2-task test.approved.txt

@@ -0,0 +1 @@
+Success. Line found OK. No error reported.