Skip to content

Build tests from dashing output#45

Closed
michaelblyons wants to merge 4 commits intoSublimeText:masterfrom
michaelblyons:test/from-dashing
Closed

Build tests from dashing output#45
michaelblyons wants to merge 4 commits intoSublimeText:masterfrom
michaelblyons:test/from-dashing

Conversation

@michaelblyons
Copy link
Copy Markdown
Collaborator

Closes #44

The defaultdict output does not appear to be stable,
so we sort the keys.

Also, it doesn't account for existing exclusion tests.
@FichteFoll
Copy link
Copy Markdown
Member

FichteFoll commented May 1, 2026

Generating a test case/assertion for each index item seems a bit much imo. I understand that the purpose is to have regression tests, but it still needs to be updated periodically whenever changes to the docs are made because we are not verifying extra index entries.

A better workflow I have in mind is to instead store a snapshot of all currently known index elements in either a tab- or comma-separated value table file with a deterministic order (and with duplicates). We would then have a single test case that validates whether the dashing output (or the sqlite db) matches this file and fail if it does not.

This is accompanied by a github action that runs on PRs opened by dependabot where this snapshot file is re-generated and committed on the same branch (if it differs). This would allow us to immediately review the diff for the automatically opened PR as a simple sanity check without requiring any updating of tests while also being a more thorough verification (catches additions and duplicates alike).

What do you think?

@michaelblyons
Copy link
Copy Markdown
Collaborator Author

No objection in principle.

I do think that this schema makes more sense for the format than one line per duplicate:

Path1 Entity type Search text Count
docs/foo.html Class sublime.Foo 1

Do you want this before merging 4205 to get the diff, or send that through?

Footnotes

  1. No #autolink-123 anchor. I don't think the autolink numbers are stable, and that would make the diffs noisy.

@FichteFoll
Copy link
Copy Markdown
Member

I do not need to see the diff, so we can merge 4205 first and then work on the diff.

It would have been be a good opportunity to test the entire setup since the docs do not change that often, but the PR also already exists, so I do not really mind. In order to test the action more easily, we may want to run the action that updates the snapshot on all merge requests and not only those opened by dependabot.

@michaelblyons michaelblyons deleted the test/from-dashing branch May 1, 2026 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make index inclusion tests from dashing output

2 participants