search

jolly-good-toolbelt / sphinx_gherkindoc

A tool to convert Gherkin into Sphinx documentation
https://jolly-good-toolbelt.github.io/sphinx_gherkindoc/
11 stars 10 forks source link

Ensure TOC tree matches actual files #44

Closed rbcasperson closed 4 years ago

rbcasperson commented 4 years ago

Fixes #43

Some details for the issue are in well, the linked issue. The solution here is basically to wait to create the toc tree file until we know which feature rst files are actually created. Then the toc tree can match what is there.

One side effect of the recent changes in 3.5.0 (#40) is that if a feature file exists, but does not have any scenarios in it, it will be excluded. That happens even if you aren't using the new flags --include-tags or --exclude-tags. My feeling is that it is odd to have a feature file with no scenarios, but if it does exist, we probably should at least document the feature part. So in the 2nd commit, I added that logic to make sure the 3.4.* default logic is reinstated. There could be an argument for not having the last commit, so let me know if we need to figure that out.

One big thing for this PR is that we don't currently have any tests for the cli.py logic. That would involve more of an end to end test. Rather than adding all that new test setup in (not sure how easy/difficult it will be), I prioritized fixing the bug. I manually tested this in my environment and it fixes the toc tree issue. I'd like for the reviewers to test in their respective environments as well to make sure it is good. I wouldn't mind a FF to add in tests, but again I'm not sure how much work that will be.

dgou commented 4 years ago

Not sure how I feel about cli tests. My higher level thoughts are that this code probably needs some restructuring to make it more testable, but that is probably non-trivial work.

rbcasperson commented 4 years ago

Thanks for the review @dgou. Yeah I agree it probably makes more sense to refactor the logic in CLI so that layer is really thin and everything else is really testable.