Issue#1241: Reduced duplication in token classification tests

[JHertz5]

Resolves #1241.

This PR replaces a set of explicitly defined tests that share identical code with a single test that generates test cases for each test. The list of subdirectories in the directory is used to build the list of test cases.

It is possible to use the list of classifier source files as the source of the test cases instead. The advantage of this would be that, when a new token is introduced, it will automatically have a test case generated for it, and the test case will fail until the classification input and result are added, informing the user that they need to update the test cases. I think I will make this change separately, as it will require the generation of quite a few new test files to cover tokens that exist already, but don't currently have their own test cases.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.19%. Comparing base (baf22da) to head (e8a6019). Report is 46 commits behind head on master.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1242 +/- ## ========================================== + Coverage 94.01% 94.19% +0.18% ========================================== Files 1557 1602 +45 Lines 29028 29762 +734 Branches 3414 3496 +82 ========================================== + Hits 27291 28035 +744 + Misses 1303 1293 -10 Partials 434 434 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[larshb]

Will this change remove the convenience of running single tests from within VS Code (editor)?

Maybe lLrmUnits should be constantly defined and the generated list compared with it (as an additional test)?

[JHertz5]

Hi @larshb

Thanks for your feedback.

Will this change remove the convenience of running single tests from within VS Code (editor)?

I don't have the testing integration set up in my VS Code, so I can't answer that. Perhaps you can check the branch out and let us know. For the tests within test/vhdlFile/, this would replace all of the individual test files, but there is still an individual test being run for each different token. Perhaps VS Code is clever enough to work out that there are individual tests being run within the file. Either way, the whole test takes 2 seconds to run, so I'd suggest that it is worthwhile to not have to maintain 90+ identical test files. I've attached the test log from running pytest in the terminal below, so you can see how it looks:

$ pytest -v tests/vhdlFile/test_token.py 
=================================================================================================== test session starts ===================================================================================================
platform linux -- Python 3.10.12, pytest-8.3.2, pluggy-1.5.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/jhertzog/code/vhdl-style-guide-fork
configfile: pyproject.toml
collected 91 items                                                                                                                                                                                                        

tests/vhdlFile/test_token.py::TestClassification::test_access_type_definition PASSED                                                                                                                                [  1%]
tests/vhdlFile/test_token.py::TestClassification::test_aggregate PASSED                                                                                                                                             [  2%]
tests/vhdlFile/test_token.py::TestClassification::test_alias_declaration PASSED                                                                                                                                     [  3%]
tests/vhdlFile/test_token.py::TestClassification::test_architecture_body PASSED                                                                                                                                     [  4%]
tests/vhdlFile/test_token.py::TestClassification::test_assertion_statement PASSED                                                                                                                                   [  5%]
tests/vhdlFile/test_token.py::TestClassification::test_attribute_declaration PASSED                                                                                                                                 [  6%]
tests/vhdlFile/test_token.py::TestClassification::test_attribute_specification PASSED                                                                                                                               [  7%]
tests/vhdlFile/test_token.py::TestClassification::test_block_header PASSED                                                                                                                                          [  8%]
tests/vhdlFile/test_token.py::TestClassification::test_block_statement PASSED                                                                                                                                       [  9%]
tests/vhdlFile/test_token.py::TestClassification::test_case_generate_statement PASSED                                                                                                                               [ 10%]
tests/vhdlFile/test_token.py::TestClassification::test_case_statement PASSED                                                                                                                                        [ 12%]
tests/vhdlFile/test_token.py::TestClassification::test_comment PASSED                                                                                                                                               [ 13%]
tests/vhdlFile/test_token.py::TestClassification::test_component_declaration PASSED                                                                                                                                 [ 14%]
tests/vhdlFile/test_token.py::TestClassification::test_component_instantiation_statement PASSED                                                                                                                     [ 15%]
tests/vhdlFile/test_token.py::TestClassification::test_concurrent_assertion_statement PASSED                                                                                                                        [ 16%]
tests/vhdlFile/test_token.py::TestClassification::test_concurrent_conditional_signal_assignment PASSED                                                                                                              [ 17%]
tests/vhdlFile/test_token.py::TestClassification::test_concurrent_procedure_call_statement PASSED                                                                                                                   [ 18%]
tests/vhdlFile/test_token.py::TestClassification::test_concurrent_selected_signal_assignment PASSED                                                                                                                 [ 19%]
tests/vhdlFile/test_token.py::TestClassification::test_concurrent_signal_assignment_statement PASSED                                                                                                                [ 20%]
tests/vhdlFile/test_token.py::TestClassification::test_concurrent_simple_signal_assignment PASSED                                                                                                                   [ 21%]
tests/vhdlFile/test_token.py::TestClassification::test_concurrent_statement PASSED                                                                                                                                  [ 23%]
tests/vhdlFile/test_token.py::TestClassification::test_conditional_force_assignment PASSED                                                                                                                          [ 24%]
tests/vhdlFile/test_token.py::TestClassification::test_conditional_variable_assignment PASSED                                                                                                                       [ 25%]
tests/vhdlFile/test_token.py::TestClassification::test_conditional_waveform_assignment PASSED                                                                                                                       [ 26%]
tests/vhdlFile/test_token.py::TestClassification::test_configuration_declaration PASSED                                                                                                                             [ 27%]
tests/vhdlFile/test_token.py::TestClassification::test_configuration_specification PASSED                                                                                                                           [ 28%]
tests/vhdlFile/test_token.py::TestClassification::test_constant_declaration PASSED                                                                                                                                  [ 29%]
tests/vhdlFile/test_token.py::TestClassification::test_constrained_array_definition PASSED                                                                                                                          [ 30%]
tests/vhdlFile/test_token.py::TestClassification::test_constraint PASSED                                                                                                                                            [ 31%]
tests/vhdlFile/test_token.py::TestClassification::test_context_clause PASSED                                                                                                                                        [ 32%]
tests/vhdlFile/test_token.py::TestClassification::test_context_declaration PASSED                                                                                                                                   [ 34%]
tests/vhdlFile/test_token.py::TestClassification::test_context_reference PASSED                                                                                                                                     [ 35%]
tests/vhdlFile/test_token.py::TestClassification::test_entity_declaration PASSED                                                                                                                                    [ 36%]
tests/vhdlFile/test_token.py::TestClassification::test_entity_header PASSED                                                                                                                                         [ 37%]
tests/vhdlFile/test_token.py::TestClassification::test_entity_statement_part PASSED                                                                                                                                 [ 38%]
tests/vhdlFile/test_token.py::TestClassification::test_enumeration_type_definition PASSED                                                                                                                           [ 39%]
tests/vhdlFile/test_token.py::TestClassification::test_exit_statement PASSED                                                                                                                                        [ 40%]
tests/vhdlFile/test_token.py::TestClassification::test_external_name PASSED                                                                                                                                         [ 41%]
tests/vhdlFile/test_token.py::TestClassification::test_file_declaration PASSED                                                                                                                                      [ 42%]
tests/vhdlFile/test_token.py::TestClassification::test_file_type_definition PASSED                                                                                                                                  [ 43%]
tests/vhdlFile/test_token.py::TestClassification::test_for_generate_statement PASSED                                                                                                                                [ 45%]
tests/vhdlFile/test_token.py::TestClassification::test_format_ansi PASSED                                                                                                                                           [ 46%]
tests/vhdlFile/test_token.py::TestClassification::test_full_type_declaration PASSED                                                                                                                                 [ 47%]
tests/vhdlFile/test_token.py::TestClassification::test_function_specification PASSED                                                                                                                                [ 48%]
tests/vhdlFile/test_token.py::TestClassification::test_generate_statement_body PASSED                                                                                                                               [ 49%]
tests/vhdlFile/test_token.py::TestClassification::test_generic_map_aspect PASSED                                                                                                                                    [ 50%]
tests/vhdlFile/test_token.py::TestClassification::test_group_declaration PASSED                                                                                                                                     [ 51%]
tests/vhdlFile/test_token.py::TestClassification::test_if_generate_statement PASSED                                                                                                                                 [ 52%]
tests/vhdlFile/test_token.py::TestClassification::test_if_statement PASSED                                                                                                                                          [ 53%]
tests/vhdlFile/test_token.py::TestClassification::test_incomplete_type_declaration PASSED                                                                                                                           [ 54%]
tests/vhdlFile/test_token.py::TestClassification::test_integer_type_definition PASSED                                                                                                                               [ 56%]
tests/vhdlFile/test_token.py::TestClassification::test_interface_function_specification PASSED                                                                                                                      [ 57%]
tests/vhdlFile/test_token.py::TestClassification::test_interface_package_declaration PASSED                                                                                                                         [ 58%]
tests/vhdlFile/test_token.py::TestClassification::test_library_clause PASSED                                                                                                                                        [ 59%]
tests/vhdlFile/test_token.py::TestClassification::test_loop_statement PASSED                                                                                                                                        [ 60%]
tests/vhdlFile/test_token.py::TestClassification::test_next_statement PASSED                                                                                                                                        [ 61%]
tests/vhdlFile/test_token.py::TestClassification::test_null_statement PASSED                                                                                                                                        [ 62%]
tests/vhdlFile/test_token.py::TestClassification::test_package_body PASSED                                                                                                                                          [ 63%]
tests/vhdlFile/test_token.py::TestClassification::test_package_declaration PASSED                                                                                                                                   [ 64%]
tests/vhdlFile/test_token.py::TestClassification::test_package_header PASSED                                                                                                                                        [ 65%]
tests/vhdlFile/test_token.py::TestClassification::test_package_instantiation_declaration PASSED                                                                                                                     [ 67%]
tests/vhdlFile/test_token.py::TestClassification::test_pragmas PASSED                                                                                                                                               [ 68%]
tests/vhdlFile/test_token.py::TestClassification::test_preprocessor PASSED                                                                                                                                          [ 69%]
tests/vhdlFile/test_token.py::TestClassification::test_procedure_call_statement PASSED                                                                                                                              [ 70%]
tests/vhdlFile/test_token.py::TestClassification::test_procedure_specification PASSED                                                                                                                               [ 71%]
tests/vhdlFile/test_token.py::TestClassification::test_process_statement PASSED                                                                                                                                     [ 72%]
tests/vhdlFile/test_token.py::TestClassification::test_protected_type_body PASSED                                                                                                                                   [ 73%]
tests/vhdlFile/test_token.py::TestClassification::test_protected_type_declaration PASSED                                                                                                                            [ 74%]
tests/vhdlFile/test_token.py::TestClassification::test_record_type_definition PASSED                                                                                                                                [ 75%]
tests/vhdlFile/test_token.py::TestClassification::test_report_statement PASSED                                                                                                                                      [ 76%]
tests/vhdlFile/test_token.py::TestClassification::test_resolution_indication PASSED                                                                                                                                 [ 78%]
tests/vhdlFile/test_token.py::TestClassification::test_return_statement PASSED                                                                                                                                      [ 79%]
tests/vhdlFile/test_token.py::TestClassification::test_selected_force_assignment PASSED                                                                                                                             [ 80%]
tests/vhdlFile/test_token.py::TestClassification::test_selected_variable_assignment PASSED                                                                                                                          [ 81%]
tests/vhdlFile/test_token.py::TestClassification::test_selected_waveform_assignment PASSED                                                                                                                          [ 82%]
tests/vhdlFile/test_token.py::TestClassification::test_sign PASSED                                                                                                                                                  [ 83%]
tests/vhdlFile/test_token.py::TestClassification::test_signal_declaration PASSED                                                                                                                                    [ 84%]
tests/vhdlFile/test_token.py::TestClassification::test_simple_force_assignment PASSED                                                                                                                               [ 85%]
tests/vhdlFile/test_token.py::TestClassification::test_simple_release_assignment PASSED                                                                                                                             [ 86%]
tests/vhdlFile/test_token.py::TestClassification::test_simple_variable_assignment PASSED                                                                                                                            [ 87%]
tests/vhdlFile/test_token.py::TestClassification::test_simple_waveform_assignment PASSED                                                                                                                            [ 89%]
tests/vhdlFile/test_token.py::TestClassification::test_subprogram_body PASSED                                                                                                                                       [ 90%]
tests/vhdlFile/test_token.py::TestClassification::test_subprogram_instantiation_declaration PASSED                                                                                                                  [ 91%]
tests/vhdlFile/test_token.py::TestClassification::test_subtype_declaration PASSED                                                                                                                                   [ 92%]
tests/vhdlFile/test_token.py::TestClassification::test_todo PASSED                                                                                                                                                  [ 93%]
tests/vhdlFile/test_token.py::TestClassification::test_unary_operator PASSED                                                                                                                                        [ 94%]
tests/vhdlFile/test_token.py::TestClassification::test_unbounded_array_definition PASSED                                                                                                                            [ 95%]
tests/vhdlFile/test_token.py::TestClassification::test_use_clause PASSED                                                                                                                                            [ 96%]
tests/vhdlFile/test_token.py::TestClassification::test_variable_assignment_statement PASSED                                                                                                                         [ 97%]
tests/vhdlFile/test_token.py::TestClassification::test_variable_declaration PASSED                                                                                                                                  [ 98%]
tests/vhdlFile/test_token.py::TestClassification::test_wait_statement PASSED                                                                                                                                        [100%]

Maybe lLrmUnits should be constantly defined and the generated list compared with it (as an additional test)?

Maybe I'm missing something, but what would the benefit be to this? It seems like a drawback to have to maintain and manually update a list of strings, when the list could just be automatically constructed, especially if you have to create a test to check your test! If there is some benefit that I'm not seeing, please let me know.

[larshb]

Good evening @JHertz5 I'll give it a try.

Only drawback off the top of my head would be the "hidden" side-effect of adding directories.

[larshb]

It does find the tests, but clicking the test from within the editor runs all of them.

All of them runs in less than 2 seconds combined, as you said, so this isn't really an issue.

Looks good!

[JHertz5]

Hi @larshb

Only drawback off the top of my head would be the "hidden" side-effect of adding directories.

What if we changed the directories to have some prefix that we could check against, e.g. "test_", and used that to determine whether a directory generates a test case? This would greatly reduce the likelihood of directories being accidentally included.

[larshb]

Hi @JHertz5

What if we changed the directories to have some prefix that we could check against, e.g. "test_", and used that to determine whether a directory generates a test case? This would greatly reduce the likelihood of directories being accidentally included.

Sounds like a good idea, then you wouldn't have to filter out __pycache__ either.

Edit: I'm not sure you can use os.scandir for this purpose, but you could use pathlib.Path(__file__).parent.glob("test_*") and then .is_dir() on the iterator. Or maybe even just pathlib.Path(__file__).parent.glob("test_*/")

[JHertz5]

Hi @larshb

Edit: I'm not sure you can use os.scandir for this purpose, but you could use pathlib.Path(__file__).parent.glob("test_*") and then .is_dir() on the iterator. Or maybe even just pathlib.Path(__file__).parent.glob("test_*/")

This is an excellent suggestion, thanks very much! I've never used pathlib before, it's very useful! I decided in the end not to change the directory names and to just check for directories that contain a file called ""classification_test_input.vhd", since that requires fewer changes and feels more obvious. I ended up with the following:

def get_lrm_unit_paths():
    lTestInputPaths = pathlib.Path(__file__).parent.glob("*/" + sTestInputFileName)
    lReturn = [path.parent for path in lTestInputPaths]
    return lReturn

I believe that since the objects are in a function now, they will automatically be cleaned up when the function completes, so I don't think that there's any more need for the with.

Instead of getting parsing a list of paths down to a list of LRM names, and then constructing the paths again in the test function, I decided to just pass the Path objects through to the test function. I think it's made the code much cleaner. Please let me know if you have any comments/suggestions.

Thanks again!

[larshb]

Looks good to me!

[jeremiah-c-leary]

Evening @JHertz5 and @larshb ,

Thanks for collaborating on this test improvement. It really simplifies all the classification testing.

I will be merging this to master.

Regards,

--Jeremy