Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues running scip test #283

Open
matthewnitschke-wk opened this issue Sep 30, 2024 · 1 comment
Open

Issues running scip test #283

matthewnitschke-wk opened this issue Sep 30, 2024 · 1 comment

Comments

@matthewnitschke-wk
Copy link
Contributor

In the newly added scip test subcommand, there was an additional test condition added to ensure that each file in the project folder has a correlating document within the SCIP index. This is a great thing to validate, but it seems like there's a few rough edges in its current implementation.

What I'm seeing is that directories that hold dependencies (.node_modules, .dart_tool, ...) are not default ignored, meaning this test case will always fail assuming your indexer doesn't index these directories. Additionally, scip-dart is not currently indexing both pubspec.yaml and pubspec.lock files, which are also being caught under this same error.

I see a few options for remediating the problem:

  • Support an ignore glob parameter, similar to the existing filter flag (we might want to consider renaming filter to include if we go this route, in an effort to be consistent)

    • This could be leverage at the consumer level, to selectively ignore the files/directories which are intentionally not indexed: scip test ./test --ignore=./node_modules/**
    • This is probably the most straightforward solution to the problem but does require additional configuration on the consumer side of things
  • Respect the .gitignore within the repo.

    • I would imagine that most directories that are not indexed, are already gitignored within the repo. We could pull from this gitignore file to determine whether the file in question should be validated against the indexed SCIP index
    • We'd have to ascend directories looking for .gitignore files, and there's some unique logic in the gitignore file that might make this approach not very straightforward
  • Hardcode indexers and the files that should be ignored

    • I don't like this approach, the scip cli should probably continue to be language unaware, but hardcoding the directories/files that each indexer should ignore is a potential solution to the problem
  • Downgrade this specific test case to a warning

    • Instead of exit 0ing this failure case, the command could consider it a warning condition, and log the issue
  • Move this condition/test case to a different subcommand

    • I envisioned scip test being mostly used for opt-in functionality, selective testing a index.scip file, validating specific indexer functionality. The addition of a default condition that always validates each file has a correlating document goes slightly against this goal. The test case itself could be added as a flag condition to scip test (scip test --check-documents), or added to other commands (like scip lint/scip snapshot)
@varungandhi-src
Copy link
Contributor

Respect the .gitignore within the repo.

I think this is a good default more generally. We can have a --no-ignore flag similar to ripgrep for skipping this. There is a library which might make implementing this easy - https://github.com/boyter/gocodewalker

The addition of a default condition that always validates each file has a correlating document goes slightly against this goal. The test case itself could be added as a flag condition to scip test (scip test --check-documents),

I think this option seems like the simplest one, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants