Repository: obsidian-tasks-group/obsidian-tasks (ts)

Issue:
Make StatusRegistry easier to test
### Description

The StatusRegistry is a singleton and can only be accessed modified via a getInstance() method.

https://github.com/obsidian-tasks-group/obsidian-tasks/blob/5c5d28850045d38dca8a7fe90b9ab7c175077bdf/src/StatusRegistry.ts#L3-L22

This makes for convoluted test code, as every test needs to:

- reset the global state at the start, just in case someone else didn't
- restore it at the end, to be a good citizen




### Required Steps

I propose to retain the static instance, for minimal changes to the plugin code, but make the constructor public, to simplify the tests and also to allow production code to say things like 'give me the default statuses please'

And the simplify the existing tests.

### How to Test

_No response_

Pull request description:
refactor: Make StatusRegistry easier to test
<!--- Provide a general summary of your changes in the Title above -->

# Description

Make StatusRegistry easier to test.

The constructor is now public, so that tests can be written without changing global state.

Docs and comments added to explain this in the code.

## Motivation and Context

Fixes #1485.

## How has this been tested?

By running the existing tests.

Helpfully, the one that tested toggling of non-standard statuses initially failed, showing that it should be adjusting the global StatusRegistry. Comments added to clarify this.

## Screenshots (if appropriate)

## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->


Internal changes:

- [x] **Refactor** (prefix: `refactor` - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)

## Checklist

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [x] My code follows the code style of this project and passes `yarn run lint`.
- [ ] My change requires a change to the documentation.
- [ ] I have [updated the documentation](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md#updating-documentation) accordingly.
- [x] My change has adequate [Unit Test coverage](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md#maintaining-the-tests).

## Terms

<!--
By submitting this pull request, you must agree to follow our
[contributing guide](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md) and
[Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md).
Put an x in the boxes to confirm you agree.
-->

- [x] My contribution follow this project's [contributing guide](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CONTRIBUTING.md)
- [x] I agree to follow this project's [Code of Conduct](https://github.com/obsidian-tasks-group/obsidian-tasks/blob/main/CODE_OF_CONDUCT.md)


Patch:
diff --git a/src/StatusRegistry.ts b/src/StatusRegistry.ts
index c3db5498..3dff2f3e 100644
--- a/src/StatusRegistry.ts
+++ b/src/StatusRegistry.ts
@@ -3,6 +3,16 @@ import { Status, StatusConfiguration } from './Status';
 /**
  * Tracks all the registered statuses a task can have.
  *
+ * There are two ways of using this class.
+ * - In 'production' code, that is in the actual plugin code that is released,
+ *   call `StatusRegistry.getInstance()` to obtain the single global instance.
+ *   Any changes to the statuses in that instance are reflected everywhere throughout
+ *   the plugin.
+ *   For example, the code to toggle task statuses use the global instance.
+ * - Tests of StatusRegistry capabilities do not need to modify the global instance:
+ *   They should use `new StatusRegistry()`, which makes for simpler, more readable
+ *   tests that can be run in parallel.
+ *
  * @export
  * @class StatusRegistry
  */
@@ -15,9 +25,12 @@ export class StatusRegistry {
      * Creates an instance of Status and registers it for use. It will also check to see
      * if the default todo and done are registered and if not handle it internally.
      *
+     * Code in the plugin should use {@link getInstance} to use and modify the global
+     * StatusRegistry.
+     *
      * @memberof StatusRegistry
      */
-    private constructor() {
+    public constructor() {
         this.addDefaultStatusTypes();
     }
 


Test patch:
diff --git a/tests/StatusRegistry.test.ts b/tests/StatusRegistry.test.ts
index 639eca5c..a62bc2a3 100644
--- a/tests/StatusRegistry.test.ts
+++ b/tests/StatusRegistry.test.ts
@@ -10,6 +10,9 @@ jest.mock('obsidian');
 window.moment = moment;
 
 describe('StatusRegistry', () => {
+    // Reset the global StatusRegistry before and after each test.
+    // The global StatusRegistry is used by the code that toggles tasks.
+    // Where possible, tests should create their own StatusRegistry to act on.
     beforeEach(() => {
         StatusRegistry.getInstance().clearStatuses();
     });
@@ -22,7 +25,7 @@ describe('StatusRegistry', () => {
         // Arrange
 
         // Act
-        const statusRegistry = StatusRegistry.getInstance();
+        const statusRegistry = new StatusRegistry();
         const doneStatus = statusRegistry.byIndicator('x');
 
         // Assert
@@ -42,8 +45,8 @@ describe('StatusRegistry', () => {
 
     it('should allow task to toggle through standard transitions', () => {
         // Arrange
-        const statusRegistry = StatusRegistry.getInstance();
-        statusRegistry.clearStatuses();
+        // Global statusRegistry instance - which controls toggling - will have been reset
+        // in beforeEach() above.
         const line = '- [ ] this is a task starting at A';
         const path = 'file.md';
         const sectionStart = 1337;
@@ -77,8 +80,8 @@ describe('StatusRegistry', () => {
 
     it('should allow task to toggle from cancelled to todo', () => {
         // Arrange
-        const statusRegistry = StatusRegistry.getInstance();
-        statusRegistry.clearStatuses();
+        // Global statusRegistry instance - which controls toggling - will have been reset
+        // in beforeEach() above.
         const line = '- [-] This is a cancelled task';
         const path = 'file.md';
         const sectionStart = 1337;
@@ -105,7 +108,7 @@ describe('StatusRegistry', () => {
 
     it('should allow lookup of next status for a status', () => {
         // Arrange
-        const statusRegistry = StatusRegistry.getInstance();
+        const statusRegistry = new StatusRegistry();
         statusRegistry.clearStatuses();
         const statusA = new Status(new StatusConfiguration('a', 'A', 'b', false));
         const statusB = new Status(new StatusConfiguration('b', 'B', 'c', false));
@@ -127,7 +130,7 @@ describe('StatusRegistry', () => {
 
     it('should handle adding custom StatusConfiguration', () => {
         // Arrange
-        const statusRegistry = StatusRegistry.getInstance();
+        const statusRegistry = new StatusRegistry();
         const statusConfiguration = new StatusConfiguration('a', 'A', 'b', false);
         statusRegistry.add(statusConfiguration);
 
@@ -138,7 +141,7 @@ describe('StatusRegistry', () => {
 
     it('should not modify an added custom Status', () => {
         // Arrange
-        const statusRegistry = StatusRegistry.getInstance();
+        const statusRegistry = new StatusRegistry();
         const status = new Status(new StatusConfiguration('a', 'A', 'b', false));
         statusRegistry.add(status);
 
@@ -149,16 +152,17 @@ describe('StatusRegistry', () => {
 
     it('should allow task to toggle through custom transitions', () => {
         // Arrange
-        const statusRegistry = StatusRegistry.getInstance();
-        statusRegistry.clearStatuses();
+        // Toggling code uses the global status registry, so we must explicitly modify that instance.
+        // It will already have been reset in beforeEach() above.
+        const globalStatusRegistry = StatusRegistry.getInstance();
         const statusA = new Status(new StatusConfiguration('a', 'A', 'b', false));
         const statusB = new Status(new StatusConfiguration('b', 'B', 'c', false));
         const statusC = new Status(new StatusConfiguration('c', 'C', 'd', false));
         const statusD = new Status(new StatusConfiguration('d', 'D', 'a', false));
-        statusRegistry.add(statusA);
-        statusRegistry.add(statusB);
-        statusRegistry.add(statusC);
-        statusRegistry.add(statusD);
+        globalStatusRegistry.add(statusA);
+        globalStatusRegistry.add(statusB);
+        globalStatusRegistry.add(statusC);
+        globalStatusRegistry.add(statusD);
         const line = '- [a] this is a task starting at A';
         const path = 'file.md';
         const sectionStart = 1337;