Make tests that use global filter less repetitive and safe against test failures within the test

[claremacrae]

Expected Behavior

That jest tests that require the Tasks global filter to be set are elegant, and guaranteed to clean up after themselves.

Current behaviour

This issue is about this pattern of lines, that appears frequently throughout the unit tests:

        const originalSettings = getSettings();
        updateSettings({ globalFilter: 'global-filter' });
...
        // Cleanup
        updateSettings(originalSettings);

Problems with this include:

• It's boiler plate code that has to be copied repeatedly
• It obscures the actual test
• It's easy to forget the cleanup section
• If there is a test failure before the cleanup section, the settings don't get reverted, and subsequent tests expecting different settings will fail.

For example, here are two example tests that use different global filters:

    it('with tag as global filter - all tags included', () => {
        // Arrange
        const originalSettings = getSettings();
        updateSettings({ globalFilter: '#task' });

        const task = fromLine({
            line: '- [ ] #task Initial  description  ⏫  #tag1 ✅ 2022-08-12 #tag2/sub-tag ',
        });

        // Act, Assert
        // Global filter tag is removed:
        expect(field.value(task)).toStrictEqual(
            'Initial  description #tag1 #tag2/sub-tag',
        );

        // Cleanup
        updateSettings(originalSettings);
    });

    it('with non-tag as global filter - all tags included', () => {
        // Arrange
        const originalSettings = getSettings();
        updateSettings({ globalFilter: 'global-filter' });

        const task = fromLine({
            line: '- [ ] global-filter Initial  description  ⏫  #tag1 ✅ 2022-08-12 #tag2/sub-tag ',
        });

        // Act, Assert
        // Global filter tag is removed:
        expect(field.value(task)).toStrictEqual(
            'Initial  description #tag1 #tag2/sub-tag',
        );

        // Cleanup
        updateSettings(originalSettings);
    });
});

Steps to reproduce

Search for const originalSettings = getSettings(); in the tests.

Which Operating Systems are you using?

• [ ] Android
• [ ] iPhone/iPad
• [ ] Linux
• [ ] macOS
• [ ] Windows

Obsidian Version

Not applicable

Tasks Plugin Version

Source code as of 2022-08-19

Checks

• [ ] I have tried it with all other plugins disabled and the error still occurs

Possible solution

Find how jest implements pre- and post-test setup/tear down, and try to use it with some kind of decorator... Something of the following form... Note, I have guessed at what the syntax might be, based on other languages...

    @withGlobalFilter('global-filter')
    it('with non-tag as global filter - all tags included', () => {
        // Arrange
        const task = fromLine({
            line: '- [ ] global-filter Initial  description  ⏫  #tag1 ✅ 2022-08-12 #tag2/sub-tag ',
        });

        // Act, Assert
        // Global filter tag is removed:
        expect(field.value(task)).toStrictEqual(
            'Initial  description #tag1 #tag2/sub-tag',
        );
    });

[claremacrae]

On reflection, there may be tests that want to change other values in the settings, so may be a settings value should be what is used in the new mechanism, so perhaps something like this example:

{ globalFilter: 'global-filter', removeGlobalFilter: true }

[claremacrae]

I've found a nice way of doing this, that simplifies the code a lot. I'm saving my work here until I'm off a branch and can apply it systematically.

Jest's afterEach() takes care of the tidying up after each test. See https://jestjs.io/docs/setup-teardown.

So the steps would be:

1. Instigate a requirement that every test always leaves the global settings set their defaults.

2. Add a new function to Settings.ts:

   export const resetSettings = (): Settings => {
       return updateSettings(defaultSettings);
   };
   

3. At the start of every describe block that ever changes the settings, add an afterEach() call:

   describe('Sort by tags', () => {
       afterEach(() => {
           resetSettings();
       });
   

4. Remove all the per-test code for reverting the settings, which would not be called if an expect call failed and threw an exception anyway. Here is one example.

            it('should filter tag with globalFilter by tag presence when it is the global tag', () => {
                // Arrange
   -            const originalSettings = getSettings();
                updateSettings({ globalFilter: '#task' });
                const filters: Array<string> = ['tags include task'];
   
                // Act, Assert
                shouldSupportFiltering(filters, defaultTasksWithTags, []);
   -
   -            // Cleanup
   -            updateSettings(originalSettings);
            });
   

   So the end result of the above deletion of 4 lines becomes:

           it('should filter tag with globalFilter by tag presence when it is the global tag', () => {
               // Arrange
               updateSettings({ globalFilter: '#task' });
               const filters: Array<string> = ['tags include task'];
   
               // Act, Assert
               shouldSupportFiltering(filters, defaultTasksWithTags, []);
           });
   

5. Document this pattern in CONTRIBUTING.md, noting the text in 1. above.

[claremacrae]

I tried using this approach in TagsField.test.ts and unfortunately the settings were not tidied up after all the test.concurrent.each calls...

It looks to me like this cause is this: https://github.com/facebook/jest/issues/7997

test.concurrent ignores beforeEach and afterEach