Char tests

[randyzwitch]

Per #9493, wrote tests for base/char.jl since the coverage is pretty
low. This should be tests for all but bitwise operations.

[jakebolewski]

Great! Could you wrap this in a let block so the test variables do not leak scope?

[randyzwitch]

Can I just put let at the top of the file and end at the bottom? Or do I need to do let on every variable?

[nalimilan]

I think only at the top and bottom.

While you're at it, could you add tests for a few non-ASCII characters in different Unicode planes?

[nalimilan]

Though you're certainly right to test it, this behavior doesn't make any sense to me. Can we deprecate this function in 0.4? Or should we get rid of it as part of #1470?

[randyzwitch]

@nalimilan: Yes, I can add a few unicode planes as well

[kmsquire]

I'm pretty sure that if any variables used in the let block have been previously defined (globally), the previous definition will leak into the let block. This has caused testing problems previously (though it's tough to find the exact issue).

[randyzwitch]

@kmsquire Is this a suggestion to not define global variables, but rather put the arrays on each for loop?

[kmsquire]

That was in response to the suggestion to put everything in a let block,
but not declare the variables. If all test code uses let blocks, then I'm
pretty sure no global variables will be defined (beyond what is defined by
default) (not tested). But any global variables defined outside of let
blocks in other test files have the potential to leak into this module. So
if, e.g., a variable you use is already defined elsewhere, it may already
have a type and a value, which has caused problems in the past.

I remember that big being hard to track down, because it only occurred when
two particular tests were run on the same worker, and the distribution of
tests among workers is somewhat random.

One solution is to define all local variables as let variables. Another is
to be vigilant about making sure all test files use let blocks and make
sure no global variables are defined. The former seems better to me, as it
ensures the code is actually self contained and independent of decisions
made in other test files.

On Tuesday, February 3, 2015, Randy Zwitch notifications@github.com wrote:

@kmsquire https://github.com/kmsquire Is this a suggestion to not
define global variables, but rather put the arrays on each for loop?

—
Reply to this email directly or view it on GitHub
#10054 (comment).

[JeffBezanson]

One thing to note is that if an entire test file is wrapped in a let block, we don't give an accurate line number for the failing test. That should be fixed before wrapping every test file this way.

[kmsquire]

@randyzwitch, here's a more concrete example of what I've been saying:

julia> const a = 20.0
20.0

julia> let
          a=10
       end
10

julia> a
10

I don't know if there was an issue open, but here's a note of the previous problem I alluded to when variables have been previously defined globally and then used.

[johnmyleswhite]

I find it helpful to make a new module for every test file, since this fixes the scope issue without so much repetition.

[nalimilan]

Thanks for adding Unicode chars!

[randyzwitch]

So where do we fall out on this? I have the time and desire to contribute more, and tests seem to be the easiest way for me to do it. But @JeffBezanson commented on undesirable property of let blocks in error reporting, @johnmyleswhite is talking about new modules, @kmsquire is talking about scope leaks...

Should I just keep submitting PR's and we'll sort out each one?

[ihnorton]

I would use a module wrapper for further PRs for now. With a module you'll
get correct line numbers at least, and it's easy enough to change later.

On Wed, Feb 4, 2015 at 12:43 PM, Randy Zwitch notifications@github.com
wrote:

So where do we fall out on this? I have the time and desire to contribute
more, and tests seem to be the easiest way for me to do it. But
@JeffBezanson https://github.com/JeffBezanson commented on undesirable
property of let blocks in error reporting, @johnmyleswhite
https://github.com/johnmyleswhite is talking about new modules,
@kmsquire https://github.com/kmsquire is talking about scope leaks...

Should I just keep submitting PR's and we'll sort out each one?

—
Reply to this email directly or view it on GitHub
#10054 (comment).

[ViralBShah]

Yes, modules seems like a reasonable way to go.

[tkelman]

Ref #9798 which doesn't quite work yet. For now more PR's with test additions are welcome, I don't think we have a single best answer for how to structure them yet. Though for the next one, 4-space indent would be good :)

[randyzwitch]

Juno uses 2 spaces, wasn't even something I even considered

[tkelman]

Thanks for informing me of that, now I know where to file an issue.