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

Docker environment for run tests #514

Merged
merged 4 commits into from Aug 12, 2021

Conversation

ziollek
Copy link
Contributor

@ziollek ziollek commented Jul 27, 2021

I've prepared Dockerfile which enable to run tests in isolated environment with all needed tools. I hope it helps newbies like me to verify their PR.

While i was running

make longtest 

i noticed some undeterministic behaviour (some tests ie. auth_https.tdir sometimes failed due to connection timeout to petal).
I found some problem in util/netevent.c which cause switching to reading phase before send http request due to incomplete ssl_handshake.

@gthess gthess self-assigned this Aug 2, 2021
Copy link
Member

@gthess gthess left a comment

Choose a reason for hiding this comment

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

Hi, thanks for taking the time for this!

I have some comments for changes before being able to merge.
If something is not clear let me know.

I was hunting an ssl bug in windows and came to the same conclusion for the ssl handshake. May I ask how you stumbled upon this?

.gitignore Outdated Show resolved Hide resolved
util/iana_ports.inc Outdated Show resolved Hide resolved
configure Outdated Show resolved Hide resolved
doc/README.tests Outdated Show resolved Hide resolved
util/netevent.c Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@ziollek
Copy link
Contributor Author

ziollek commented Aug 4, 2021

Hi, thanks for taking the time for this!

I have some comments for changes before being able to merge.
If something is not clear let me know.

I was hunting an ssl bug in windows and came to the same conclusion for the ssl handshake. May I ask how you stumbled upon this?

Thank You for review my PR. I observed problem with handshake while launching longtests in docker, i started digging and found ssl connection was waiting for response despite request wasn't send due to not finished handshake.

I merged actual master to my branch and verified that change ca67691 fixed also above problem - so i've reverted my changes in netevent.c. I've adjusted my code to other suggestions. Please let me know if i can improve something else.

Best regards.

@gthess
Copy link
Member

gthess commented Aug 4, 2021

Thanks, looks good to me!
Currently we are in code freeze for the upcoming release but I will merge this right after that.

Something that just came to mind, I am not sure about the portability of the -E option to grep. In most man pages I find online I see support for it but we will find out when testing on different platforms for the release. Worst case we convert it back to regular regular expression.

@ziollek
Copy link
Contributor Author

ziollek commented Aug 5, 2021

Thanks for information :)

@gthess gthess changed the title Docker environment for run tests + enhancement for ssl_handshake Docker environment for run tests Aug 12, 2021
@gthess gthess merged commit 3829faf into NLnetLabs:master Aug 12, 2021
gthess added a commit that referenced this pull request Aug 12, 2021
- Merge PR #514, from ziollek: Docker environment for run tests.
gthess added a commit that referenced this pull request Aug 12, 2021
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.

None yet

2 participants