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

add some doctest to test #22287

Merged
merged 2 commits into from
Jun 26, 2017
Merged

add some doctest to test #22287

merged 2 commits into from
Jun 26, 2017

Conversation

KristofferC
Copy link
Member

No description provided.

@KristofferC KristofferC added backport pending 0.6 docs This change adds or pertains to documentation labels Jun 8, 2017
julia> @test foo("bar") == 9
Test Passed
Expression: foo("bar") == 9
Evaluated: 9 == 9

Copy link
Contributor

Choose a reason for hiding this comment

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

are all the extra blank lines needed for this to pass?

Copy link
Member Author

Choose a reason for hiding this comment

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

They exist in the output.

Copy link
Contributor

Choose a reason for hiding this comment

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

is that a bug?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if you could call it a bug but it is not pretty at least.

Copy link
Contributor

Choose a reason for hiding this comment

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

looks wrong, anyone know what's causing it?

Copy link
Member

Choose a reason for hiding this comment

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

diff --git a/base/test.jl b/base/test.jl
index c841552..25ab49f 100644
--- a/base/test.jl
+++ b/base/test.jl
@@ -69,9 +69,9 @@ struct Pass <: Result
     value
 end
 function Base.show(io::IO, t::Pass)
-    print_with_color(:green, io, "Test Passed\n"; bold = true)
+    print_with_color(:green, io, "Test Passed"; bold = true)
     if !(t.orig_expr === nothing)
-        print(io, "  Expression: ", t.orig_expr)
+        print(io, "\n  Expression: ", t.orig_expr)
     end
     if t.test_type == :test_throws
         # The correct type of exception was thrown

?

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, makes sense.

@kshyatt
Copy link
Contributor

kshyatt commented Jun 21, 2017

Can we merge this?

@KristofferC
Copy link
Member Author

Probably.

@KristofferC
Copy link
Member Author

Pushed @fredrikekre commit and updated doctests. Should be good to go.

@@ -68,23 +68,25 @@ julia> @test foo(:cat) == 1
Error During Test
Test threw an exception of type MethodError
Expression: foo(:cat) == 1
MethodError: `length` has no method matching length(::Symbol)
Copy link
Contributor

Choose a reason for hiding this comment

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

should this block go in the same jldoctest set?

Copy link
Member Author

Choose a reason for hiding this comment

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

No because the stacktrace is very weird from how Documenter evaluates things in its own module.

julia> @test_throws MethodError foo(:cat)
Test Passed
Expression: foo(:cat)
Evaluated: MethodError

Copy link
Contributor

Choose a reason for hiding this comment

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

should this blank line still be here?

in do_test at test.jl:53
Test Failed
Expression: 1 ≈ 0.999999
Evaluated: 1 isapprox 0.999999
Copy link
Contributor

Choose a reason for hiding this comment

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

likely needs updating for #22296

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I messed up when running the doctests (they didnt actually get run) so I will rebase master and make sure everything passes.

Copy link
Member

Choose a reason for hiding this comment

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

Actually this was corrected as part of #22315

@KristofferC
Copy link
Member Author

Doctests for this file now passes locally.

Copy link
Member

@Sacha0 Sacha0 left a comment

Choose a reason for hiding this comment

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

lgtm! :)

@KristofferC KristofferC merged commit 2796b40 into master Jun 26, 2017
@tkelman tkelman deleted the kc/doctest_help branch June 26, 2017 19:48
ararslan pushed a commit that referenced this pull request Sep 11, 2017
* fix spurious newlines in Base.Test printing

* add some doctest to test

Ref #22287
(cherry picked from commit 2796b40)
ararslan pushed a commit that referenced this pull request Sep 13, 2017
* fix spurious newlines in Base.Test printing

* add some doctest to test

(cherry picked from commit 2796b40)
vtjnash pushed a commit that referenced this pull request Sep 14, 2017
* fix spurious newlines in Base.Test printing

* add some doctest to test

(cherry picked from commit 2796b40)
ararslan pushed a commit that referenced this pull request Sep 15, 2017
* fix spurious newlines in Base.Test printing

* add some doctest to test

(cherry picked from commit 2796b40)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants