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 spawn test when env BAR is set #39214

Merged
merged 1 commit into from
Jan 13, 2021
Merged

Fix spawn test when env BAR is set #39214

merged 1 commit into from
Jan 13, 2021

Conversation

omus
Copy link
Member

@omus omus commented Jan 12, 2021

When running the "spawn" tests I noticed this failure:

spawn                              (7) |         failed at 2021-01-12T14:23:55.599
Test Failed at /Users/omus/Development/Julia/x86/latest-src/usr/share/julia/test/spawn.jl:718
  Expression: strip(String(read(cmd))) == "foo"
   Evaluated: "foo 2" == "foo"

I discovered that the issue was that I had set BAR=2 in my shell environment before starting Julia and running the tests. The change here ensures that the test doesn't rely on the user not defining BAR.

One thing to note. This issue only seems to appear when you have compiled the Julia dependencies from source and doesn't occur when using BinaryBuilder dependencies.

@omus omus added the test This change adds or pertains to unit tests label Jan 12, 2021
@omus
Copy link
Member Author

omus commented Jan 12, 2021

Reproducing the failure can be done with:

export BAR=2
julia -e '
    using Test
    shcmd = "sh"
    withenv("FOO" => "foo") do
        cmd = Cmd(`$shcmd -c "echo \$FOO \$BAR"`)
        @test strip(String(read(cmd))) == "foo"
    end'

@omus omus requested a review from staticfloat January 13, 2021 17:21
@staticfloat staticfloat merged commit c3acba1 into master Jan 13, 2021
@staticfloat staticfloat deleted the cv/spawn-test-fix branch January 13, 2021 18:53
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants