Member

Look good. As noted in the Contributing Guide, this needs tests and an update to the Release Notes. Add those and I'll merge it.

Author

Concerning Release Notes - forgot those, will add that in a minute.

Concerning tests - from the Contributing Guide:

While it is generally fine to leave those tests for the Travis server to run when a pull request is submitted, for more advanced changes, you may want to run those tests locally.

I don't consider this as an advanced change, and the tests on the Travis server have all passed, so which other tests are needed?

Member

You need to add tests which confirm that the new option works as it is supposed to. Does it actually change the title when set? Does the default actually get used when not set?

I haven't checked, but the second question above might already get answered in the existing tests. If so, you would only need to add at least one new test. If not, then you would need to add at least two.

Member

Note that as per the tests policy, any new tests should go in /tests/test_syntax/extensions. As none of the TOC tests have been migrated to the new system, you'll need to add a new file test_toc.py there and create your tests. At a minimum, you should create the two tests which confirm the default and a custom title get used.

Note that the existing tests for the TOC extension are in tests/extensions (in various files with names starting with toc) and the TestTOC class of tests/test_extensions.py. If any of those tests cover the same behavior, they should be converted to the new style tests and removed from here. I'm thinking perhaps the testPermalink tests in the TestTOC class. You don't have to covert any other tests, but are certainly welcome to.

Author

Uh, I have zero experience in testing and have no clue what those files do. Maybe you could add those for me? Sorry :(

Member

That's fine. However, I have very little time to do it. So unless someone else wants to do the work it will likely be some time before I get to it. I am not going to merge this (or anything else) without the tests.

Member

As this was on a master branch, it was easier to just create a new PR than try to push changes to this. Therefore, this is being closed in favor of #886 which uses this as a base, resolves the conflicts and adds a test.