search

AstuteSource / chasten

:dizzy: Chasten Uses XML and XPATH to Check a Python Program's AST for Specified Patterns!
https://pypi.org/project/chasten/
GNU General Public License v2.0
7 stars 8 forks source link

Test case that uses source code of `chasten` fails in development branch #59

Closed simojo closed 10 months ago

simojo commented 11 months ago

Describe the bug When on a development branch, after having made some changes to the source code, I was running tests and realized that chasten is being called to analyze itself:

chasten analyze testing --search-path /path/to/chasten/repo --config /path/to/chasten/repo/.chasten --verbose

The problem with this is that chasten is using a hard-coded config located in the .chasten directory, and because chasten's code changes, these checks will not always pass.

.chasten
├── checks.yml
└── config.yml

This hard-coded config was being read by the test case tests/test_main.py::test_cli_analyze_correct_arguments_analyze_chasten_codebase. Chasten is essentially shooting itself in the foot, and will not pass test cases when significant changes are made to it that break the following checks:

checks:
  - name: "class-definition"
    code: "CDF"
    id: "C001"
    pattern: './/ClassDef'
    count:
      min: 1
      max: 50
  - name: "all-function-definition"
    code: "AFD"
    id: "F001"
    pattern: './/FunctionDef'
    count:
      min: 1
      max: 200
  - name: "non-test-function-definition"
    code: "NTF"
    id: "F002"
    pattern: './/FunctionDef'
    count:
      min: 40
      max: 70
  - name: "single-nested-if"
    code: "SNI"
    id: "CL001"
    pattern: './/FunctionDef/body//If'
    count:
      min: 1
      max: 100
  - name: "double-nested-if"
    code: "DNI"
    id: "CL002"
    pattern: './/FunctionDef/body//If'
    count:
      min: 1
      max: 15

In my case, you can read the output below to see that the non-test-function-definition and double-nested-if patterns were not being found. There is no need to enforce these arbitrary checks on our code.

✨ Analyzing Python source code in: /_scratch/chasten

🎉 Performing 5 check(s):

  ✓ id: 'C001', name: 'class-definition', pattern: './/ClassDef', min=1, max=50
    • /_scratch/chasten/chasten/constants.py - 10 matches
    • /_scratch/chasten/chasten/debug.py - 2 matches
    • /_scratch/chasten/chasten/enumerations.py - 3 matches
    • /_scratch/chasten/chasten/results.py - 6 matches
    • /_scratch/chasten/chasten/server.py - 1 matches
  ✓ id: 'F001', name: 'all-function-definition', pattern: './/FunctionDef', min=1, max=200
    • /_scratch/chasten/chasten/filesystem.py - 12 matches
    • /_scratch/chasten/chasten/validate.py - 4 matches
    • /_scratch/chasten/chasten/util.py - 5 matches
    • /_scratch/chasten/chasten/main.py - 9 matches
    • /_scratch/chasten/chasten/configuration.py - 10 matches
    • /_scratch/chasten/chasten/output.py - 12 matches
    • /_scratch/chasten/chasten/checks.py - 9 matches
    • /_scratch/chasten/chasten/database.py - 5 matches
    • /_scratch/chasten/chasten/server.py - 2 matches
    • /_scratch/chasten/chasten/process.py - 4 matches
    • /_scratch/chasten/tests/test_debug.py - 8 matches
    • /_scratch/chasten/tests/test_validate.py - 4 matches
    • /_scratch/chasten/tests/test_filesystem.py - 16 matches
    • /_scratch/chasten/tests/test_configuration.py - 3 matches
    • /_scratch/chasten/tests/test_main.py - 10 matches
    • /_scratch/chasten/tests/test_util.py - 4 matches
    • /_scratch/chasten/tests/test_constants.py - 5 matches
    • /_scratch/chasten/tests/test_process.py - 3 matches
    • /_scratch/chasten/tests/test_checks.py - 8 matches
  ✗ id: 'F002', name: 'non-test-function-definition', pattern: './/FunctionDef[not(contains(@name, "test_"))]', min=40,
max=70
    • /_scratch/chasten/chasten/filesystem.py - 12 matches
    • /_scratch/chasten/chasten/validate.py - 4 matches
    • /_scratch/chasten/chasten/util.py - 5 matches
    • /_scratch/chasten/chasten/main.py - 9 matches
    • /_scratch/chasten/chasten/configuration.py - 10 matches
    • /_scratch/chasten/chasten/output.py - 10 matches
    • /_scratch/chasten/chasten/checks.py - 9 matches
    • /_scratch/chasten/chasten/database.py - 5 matches
    • /_scratch/chasten/chasten/server.py - 2 matches
    • /_scratch/chasten/chasten/process.py - 4 matches
    • /_scratch/chasten/tests/test_main.py - 1 matches
  ✓ id: 'CL001', name: 'single-nested-if', pattern: './/FunctionDef/body//If', min=1, max=100
    • /_scratch/chasten/chasten/filesystem.py - 11 matches
    • /_scratch/chasten/chasten/validate.py - 3 matches
    • /_scratch/chasten/chasten/util.py - 2 matches
    • /_scratch/chasten/chasten/main.py - 15 matches
    • /_scratch/chasten/chasten/configuration.py - 14 matches
    • /_scratch/chasten/chasten/output.py - 7 matches
    • /_scratch/chasten/chasten/checks.py - 13 matches
    • /_scratch/chasten/chasten/database.py - 11 matches
    • /_scratch/chasten/chasten/process.py - 5 matches
    • /_scratch/chasten/tests/test_filesystem.py - 1 matches
    • /_scratch/chasten/tests/test_util.py - 1 matches
    • /_scratch/chasten/tests/test_constants.py - 1 matches
    • /_scratch/chasten/tests/test_process.py - 1 matches
    • /_scratch/chasten/tests/test_checks.py - 1 matches
  ✗ id: 'CL002', name: 'double-nested-if', pattern: './/FunctionDef/body//If[ancestor::If and not(parent::orelse)]', min=1,
max=15
    • /_scratch/chasten/chasten/filesystem.py - 3 matches
    • /_scratch/chasten/chasten/validate.py - 1 matches
    • /_scratch/chasten/chasten/main.py - 2 matches
    • /_scratch/chasten/chasten/configuration.py - 2 matches
    • /_scratch/chasten/chasten/output.py - 1 matches
    • /_scratch/chasten/chasten/checks.py - 3 matches
    • /_scratch/chasten/chasten/database.py - 4 matches

To Reproduce Steps to reproduce the behavior:

  1. In chasten repo: git checkout e264ae97c4dfa3f408695158bc7fb867ea2cd769
  2. poetry lock; poetry install
  3. poetry run task test-not-randomly (this ensures no other failed test cases clutter up the output)
  4. Previously mentioned test case will fail: tests/test_main.py:118 test_cli_analyze_correct_arguments_analyze_chasten_codebase

Expected behavior This test case should pass.

Desktop (please complete the following information):

Proposed Solution

We simply need to remove this hard coded config and rely on something more static, such as a small sample python program located in tests that has code that will never change rather than analyzing our own codebase.

This is very meta that we are learning how our tool works because we're using our tool to analyze our tool, isn't it?

gkapfham commented 11 months ago

Hi @simojo, yes, I was aware of this issue. Thanks for surfacing it! Basically, it would be nice if we could use chasten during the testing of itself so that we can confirm the tool works. Is there a way in which we can "loosen" the assertions so that these tests will pass? Or, alternatively, might it be possible to remove the counts and simply confirm that the tool runs without crashing? Please let me know what you think!

simojo commented 11 months ago

@gkapfham We could leave checks in that we know will be a part of chasten (e.g. function definitions, and maybe class definitions), but I feel that we need to remove a maximum value, just so that this never becomes a problem again. I'm not sure if we can remove the counts or not. I will have to look into that.

simojo commented 10 months ago

Additionally, it appears that master is now suffering from the same problem:

 ✗ id: 'F002', name: 'non-test-function-definition', pattern: './/FunctionDef[not(contains(@name, "test_"))]', min=40, 
max=70
    • /home/runner/work/chasten/chasten/tests/test_main.py - 1 matches
    • /home/runner/work/chasten/chasten/chasten/util.py - 5 matches
    • /home/runner/work/chasten/chasten/chasten/main.py - 14 matches
    • /home/runner/work/chasten/chasten/chasten/checks.py - 9 matches
    • /home/runner/work/chasten/chasten/chasten/output.py - 10 matches
    • /home/runner/work/chasten/chasten/chasten/server.py - 2 matches
    • /home/runner/work/chasten/chasten/chasten/process.py - 4 matches
    • /home/runner/work/chasten/chasten/chasten/filesystem.py - 12 matches
    • /home/runner/work/chasten/chasten/chasten/database.py - 6 matches
    • /home/runner/work/chasten/chasten/chasten/validate.py - 3 matches
    • /home/runner/work/chasten/chasten/chasten/configuration.py - 5 matches

The total matches here totals to 71, which is just above the maximum specified. This adds weight to the importance of this issue.