Skip to content

Conversation

RyanSquared
Copy link
Contributor

Replication and improvement of #8.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 84.751% when pulling f7feefe on ChickenNuggers:use-path into d59a831 on daurnimator:master.

http/client.lua Outdated
end
end
local ok, err, errno = s:starttls(tls, deadline and (deadline-monotime()))
local ok, err, errno = s:starttls(tls, timeout)
Copy link
Owner

Choose a reason for hiding this comment

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

why is this line different?

-- pending.
--
--]]
local socket_path
Copy link
Owner

Choose a reason for hiding this comment

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

What is this block doing?
You just generate a random file name... then delete it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm double checking now but Linux systems create the return value of os.tmpname() so I need to remove that, then use the filename with .socket. I'm checking later when I get home whether I actually use the path generated in the tests. I might have to create a wrapper function for this and then remove this block.

Copy link
Owner

Choose a reason for hiding this comment

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

Why bother with adding .socket? just use the result of os.tmpname.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.003%) to 85.679% when pulling 5078c92 on ChickenNuggers:use-path into f4cb30b on daurnimator:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.003%) to 85.679% when pulling b04cecf on ChickenNuggers:use-path into f4cb30b on daurnimator:master.

-- pending.
--
--]]
local socket_path = os.tmpname()
Copy link
Owner

Choose a reason for hiding this comment

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

Why not generate one per test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a reason to? If you remove it in the finally() call, it should be done after the it() call, right?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah that's fine I guess.

@daurnimator daurnimator merged commit b04cecf into daurnimator:master Apr 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants