Er/3250 generic tests

[emmyoop]

resolves #3250

Description

Adding ability to have generic tests in the /tests directory.

• added new ParseFileType GenericTest and renamed Test to be SingularTest
• added new GenericTestParser
• parse generic tests in /tests directly after macros and before everything else.

Checklist

• I have signed the CLA
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• I have updated the CHANGELOG.md and added information about my change

[gshank]

In the long run we're probably going to want different NodeTypes for singular test and generic test. I don't know if that's something that we want to do now or not though.

[gshank]

We don't want to do this twice, once for macros and once for tests. It should be done just once, after both macros and tests are in the 'macros' dictionary.

[gshank]

This feels like a problem to me. Every file in the 'tests' directory is going to be added to both the SingularTestParser and the GenericTestParser. I'm not exactly sure what the implications are, but I think it will make partial parsing for these files impossible. I think we might have to peek into the files and allocate them to either Singular or Generic, not both. I'm going to think about this some more.

[emmyoop]

That's the main thing I don't like about this approach. It does work as I understand the tests to be structured. Singular tests end up getting ignored by generic tests because they're not in a jinja test block. And I'm purposefully ignoring generic tests in the singular test code by specifically checking to see if they're in jinja blocks.

[gshank]

File objects are saved in the 'files' dictionary under the file_id key, so the second run over the files would save a duplicated file object except with a differe parse_file_type.

We could use a different filetype for the generic tests, but longterm we may want to make "singular" tests look more like generic tests and put them in a file block, so that might not be the right path. We could also put them in a subdirectory of tests, like 'generic' (Jeremy's suggestion). We would also parse all of the tests when the test parser is processed, not at macro processing time, and have to separately do the 'macro_depends_on' for just tests. As long as the generic tests are in 'macros' prior to schema file processing it should be okay.

[jtcohen6]

My current vote is for a special directory, tests/generic. That's a little jank, but it feels like a step in the right direction, and I like it more than having generic tests defined in macros/ forever.

What Gerda and I were discussing is an eventual future where all tests are defined as blocks, some with "singular" construction and others with "generic" construction, and they could be intermixed in the same .sql files:

-- tests/my_tests.sql

{% test singular_not_null %}
  select *
  from {{ ref('orders') }}
  where id is null
{% endtest %}

{% test not_null(model, column_name) %}
  select *
  from {{ model }}
  where {{ column_name }} is null
{% endtest %}

There's history for this proposal (#2274) that goes pretty far back for all resource types (#184). Snapshots are the only real DAG node that ever made the switchover, from files to blocks. It's something we once imagined for v1.0, but deprioritized. It has the side-effect of making these queries harder, though certainly not impossible, to run in IDEs, since line/cursor location becomes important.

In the meantime, we can take an important step in that direction, without needing to get all the way there.

[emmyoop]

Tweaked it to look for a generic folder in test-paths. Is this more along the lines of how you were thinking it would work?

[gshank]

Yes, this looks good. We do end up with some generic tests having a Macro parse _file_type and some with a GenericTest parse file type. I'm not sure that's a problem, but it is something to be watchful for.

[jtcohen6]

The functional change here looks mostly good. Thanks for the flexibility in working toward a way to achieve this functionality ahead of v1.

I noticed that it was quite easy to trip a partial parsing error. Create any test in tests/generic/my_cool_test.sql, parse the project, edit the file in any way, and re-parse the project (with partial parsing enabled):

Encountered an error:
Compilation Error
  dbt found two macros named "test_my_cool_test" in the project "testy".
   To fix this error, rename or remove one of the following macros:
      - tests/generic/my_cool_test.sql
      - tests/generic/my_cool_test.sql

I only have one test there by that name, so it's an issue with not removing the test macro before scheduling it for re-parsing.

[gshank]

In core/dbt/parser/partial.py, the GenericTest filetype does not belong in the mssat_files. It needs to be handled with the 'Macro' files. There are a number of places in core/dbt/parser/partial.py where it checks for a ParseFileType of Macro. Those places also need to check for a ParseFileType of GenericTest. You could make a list of the two of them, like the mssat_files list.

[gshank]

It would probably be a good idea to add a partial parsing test with a test/generic test file to the 068_partial_parsing_tests.

[gshank]

Looks good!

[jtcohen6]

The substance of this change looks good to me.

As above, I'm seeing an issue with partial parsing. It no longer raises an exception, but every time I edit a test defined in test/generic, I see:

Partial parsing enabled but an error occurred. Switching to a full re-parse.

That's a step in the right direction, but I'd like to resolve that exception before merging. I threw a debugger into the partial parsing code, and it looks like the exception I'm tripping is:

Traceback (most recent call last):
  File "/Users/jerco/dev/product/dbt/core/dbt/parser/manifest.py", line 233, in load
    project_parser_files = self.partial_parser.get_parsing_files()
  File "/Users/jerco/dev/product/dbt/core/dbt/parser/partial.py", line 156, in get_parsing_files
    self.update_in_saved(file_id)
  File "/Users/jerco/dev/product/dbt/core/dbt/parser/partial.py", line 238, in update_in_saved
    self.update_macro_in_saved(new_source_file, old_source_file)
  File "/Users/jerco/dev/product/dbt/core/dbt/parser/partial.py", line 322, in update_macro_in_saved
    self.handle_macro_file_links(old_source_file, follow_references=True)
  File "/Users/jerco/dev/product/dbt/core/dbt/parser/partial.py", line 445, in handle_macro_file_links
    self.recursively_gather_macro_references(unique_id, referencing_nodes)
  File "/Users/jerco/dev/product/dbt/core/dbt/parser/partial.py", line 418, in recursively_gather_macro_references
    for unique_id in self.macro_child_map[macro_unique_id]:
KeyError: 'test.testy.test_my_cool_test'