-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Introduce smoke test #237
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
Introduce smoke test #237
Conversation
Oh yes yes yes! I am all in for better tests :) |
👍 ❤️ |
This checks that we render the example book correctly. In order to do that, we put a known-good copy of the book under tests/book, and then in tests/smoke.rs, we generate a copy of it, and then diff the two directories. This means that PRs that change the generated output will need to update this fixture, but it also means we get to see an easy way of what they actually update. Part of #11
Some PRs were landed in the meantime
only a small amount of change, as you can see in the second commit 😄 |
Ahh, line endings are the issue here.... I made this PR on Windows, so windows stuff succeeds, but linux stuff fails. Hrm. |
..... I am not sure how to fix this, given that if I change it the other way, it will fail on Windows. Hrm. |
Sounds like something that should be fixed in dir-diff, I filed an issue for you ❤️ |
❤️
…On Thu, Apr 27, 2017 at 2:28 PM, Carol (Nichols || Goulding) < ***@***.***> wrote:
Sounds like something that should be fixed in dir-diff, I filed an issue
for you <assert-rs/dir-diff#9> ❤️
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<https://github.com/azerupi/mdBook/pull/237#issuecomment-297799815>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABsijxwLUYqc2sGWDDgPKLCWhjDOYj6ks5r0N5rgaJpZM4MsBmp>
.
|
</div> | ||
|
||
<div id="content" class="content"> | ||
<a class="header" href="misc\introduction.html#introduction" id="introduction"><h1>Introduction</h1></a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the actual problem. Backslashes shouldn't be used in URLs. I've opened a more relevant issue: #263
Thinking about this a bit, this kind of test might be a bit too restrictive. Since the output often changes with small PRs I think it would be better to drop this idea and instead implement a lot of small tests checking for one particular property in the generated output. It would probably not catch everything, but it would be more granular than this solution. Now the major drawback would be to actually write all of the small tests. But we have had a lot of contributions recently and I think that if we make easy issues, we can get most of them contributed as mentored PRs. What do you think? |
Hey @azerupi and @steveklabnik I just noticed this PR.
I've started something similar in #374. In particular the I'd also be happy to mentor a PR for asserting particular patterns appear in the output as expected. |
Let's do it! 😄 |
This checks that we render the example book correctly. In order to do
that, we put a known-good copy of the book under tests/book, and then in
tests/smoke.rs, we generate a copy of it, and then diff the two
directories.
This means that PRs that change the generated output will need to update
this fixture, but it also means we get to see an easy way of what they
actually update.
Part of #11