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

Interpolation syntax with @tspawnat #17

Closed
wants to merge 2 commits into from

Conversation

wsphillips
Copy link

This ought to do it. Fix for #16

@tro3
Copy link
Owner

tro3 commented Jul 23, 2020

This looks good. Do you have a testcase you could add to /tests that demonstrates fail before / pass after? If not, I can generate one, and check Julia version compatibility.

@wsphillips
Copy link
Author

wsphillips commented Jul 23, 2020

I don't have a test handy, but the original implementation will fail outright if you try to pass in an interpolated variable. To prove it works, it would be something like:

function foo(x)
    sleep(0.01)
    return x
end

# With interpolation
x = 1
expect_sum = 3
t1 = @tspawnat 2 foo($x)
x += 1
t2 = @tspawnat 3 foo($x)

test_sum = fetch(t1) + fetch(t2)
@test expect_sum == test_sum

# without interpolation
x = 1
t1 = @tspawnat 2 foo(x)
x += 1
t2 = @tspawnat 3 foo(x)
test_sum = fetch(t1) + fetch(t2)
@test expect_sum == test_sum # should fail; sum will be 4

@tro3
Copy link
Owner

tro3 commented Jul 26, 2020

Okay, I have an implementation using the VERSION check, but the above test case does not properly fail without interpolation. Do you have a use case that required it? I'll try to come up with one as well.

@tro3
Copy link
Owner

tro3 commented Jul 26, 2020

Well, I guess it doesn't matter. Interpolation works with it in place, and dies without it.

@tro3 tro3 closed this in 1623a8e Jul 26, 2020
@tro3
Copy link
Owner

tro3 commented Jul 26, 2020

Okay, interpolation upgrade with test for 1.4 (but tests passing at 1.3) are in place. Revving version the Registrator now. Thanks for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants