skip @inferred tests for PkgEval

[simeonschaub]

They can also be manually disabled by setting CHAINRULES_TEST_INFERRED
to false. I have tested that all tests still pass with JULIA_PKGEVAL
set to true, do we need to test for that by spawning a new Julia
process? Once this is merged, I plan to replace occurences of
@inferred in ChainRules with @maybe_inferred as well, therefore I
exported that macro.

[oxinabox]

Possible we want:

TEST_INFERRED[] = if haskey(ENV, "CHAINRULES_TEST_INFERRED")
    parse(Bool, "CHAINRULES_TEST_INFERRED")
else
    !parse(Bool, get(ENV, "JULIA_PKGEVAL", "false"))
end

which as truth table
for C being ENV[CHAINRULES_TEST_INFERRED] and
P being JULIA_PKGEVAL
and X marking not defined

C P R
-----
X T F
X F T
F F F
F T F
T F T
T T T
T X T
F X F
X X T

over

TEST_INFERRED[] = !parse(Bool, get(ENV, "JULIA_PKGEVAL", "false")) &&
    parse(Bool, get(ENV, "CHAINRULES_TEST_INFERRED", "true"))

which has

C P R
-----
X T F
X F T
F F F
F T F
T F T
T T F
T X T
F X F
X X T

The difference is the
T T T vs T T F.
I.e. if the ChainRules specific environment variable is present, it would override the JULIA_PKGEVAL.
Which feels right to me.
If someone specifically states they want these tests to run that is stronger than if it happens to have this enviroment varaible set.

Also do we want to handle the variables not being set to parsable values?
If so could be done with try_parse + something, or the a try-catch + give an informative message.

[oxinabox]

Maybe we don't need the two env-vars.
Maybe we just need !parse(Bool, get(ENV, "JULIA_PKGEVAL", "false")) to see the default value.
And then for normal users (and for overwriting) we can document doing ChainRulesTestUtils.TEST_INFERRED[]=false / ChainRulesTestUtils.TEST_INFERRED[]=true

Maybe that is better?

[simeonschaub]

I think a separate environment variable might still be useful, since otherwise, you'd have to manually edit the testing code to disable these. I didn't really expect the two env vars to ever really be set at the same time, but just for consistency's sake I agree that your version makes more sense, so I will go with that instead if that's ok.

[oxinabox]

Can we think of a better name for this?
Might not be able to.

[nickrobinson251]

maybe_test_inferred is perhaps more accurate?

[oxinabox]

I wish @inferred has been named @test_inferred (and that it returnned a bool rather than erroring)

[simeonschaub]

I agree that it's not the greatest name, but I couldn't really think of a better one. Since it isn't an actual test I don't think it should have test in its name, but I am open to any other suggestions.

[oxinabox]

We could make it an actual test

[simeonschaub]

I think I'd prefer to make it consistent with base, so it's just a drop-in replacement for @inferred. We could think about having an @maybe_test_inferred as well, but for now this seems more straightforward to me.

[oxinabox]

do we need to test for that by spawning a new Julia process?

We do not.
But we probably should check that non-inferrable tests do pass if we disable it manually?

[oxinabox]

Needs adding to docs.
I trust your judgement wrapping things up here, and addressing the points i raised appropriately.

[simeonschaub]

I plan on merging this tomorrow afternoon if there are no objections.

[simeonschaub]

Oh, sorry, I thought I bumped the version as well, but seems like I missed it. Ok if I just do that as a commit straight to master or is the proper way to do this in its own PR?

[mzgubic]

yeah just commit the version bump