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

Fix Issue 18: Port scripts to entry points so they can be tested. #21

Merged
merged 2 commits into from
Feb 22, 2017

Conversation

jamadden
Copy link
Member

  • Ensure test coverage.

    Tests for schema2html doesn't do much validation of whether the output makes
    any sense (in multkey defaults, for example, it doesn't), just that it runs. I'll
    work on making sense in other PRs if I keep going down the direction
    of using this to produce a sphinx extension.

  • Port them to Python 3.

  • Add documentation.

Port to Python 3.

Note that this doesn't do much validation of whether the output makes
any sense (in multkey defaults, it doesn't), just that it runs. I'll
work on making sense in other PRs if I keep going down the direction
of using this to produce a sphinx extension.


@contextlib.contextmanager
def stdout_replaced(buf):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why define this here instead of using test.test_support.captured_stdout?

Of course, that's test.support.captured_stdout in Python 3.

--sigh--

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind. I see why.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's one good reason :)

Another is that I know that some Python installations installed by (Linux) package managers don't include some or all of the test package by default (you have to install an additional package); I'm never sure of exactly what may be installed by default, so I try to avoid using it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Egads, still???? Silly distributors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, gevent has run into that on numerous occasions because it uses it extensively.

Of course, that module is also explicitly documented as not being a public module and its use is discouraged. So they can make a case for it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imagine hiding something like that in the documentation. I guess those of us who've been around long enough should know to check for that rather than relying on memory.

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.

2 participants