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.

Sign up for GitHub

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

Jump to bottom

Refactor tls test for new cert command. #2676

Merged
merged 7 commits into from
Oct 22, 2018
Merged

Refactor tls test for new cert command. #2676

merged 7 commits into from
Oct 22, 2018

Conversation

srfrog
Copy link
Contributor

@srfrog srfrog commented Oct 18, 2018 •
edited by manishrjain
Loading

Closes #2615

This change is Reviewable

@srfrog srfrog requested a review from danielmai October 18, 2018 21:43
@danielmai
Copy link
Contributor

contrib/tlstest/Makefile, line 58 at r1 (raw file):

test6: cert
	@echo "Test 6: Alpha TLS reaload, Live TLS"

"reload"

danielmai
danielmai reviewed Oct 20, 2018
View reviewed changes
Copy link
Contributor

@danielmai danielmai left a comment

Choose a reason for hiding this comment

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

Reviewed 17 of 18 files at r1.
Reviewable status: 17 of 18 files reviewed, 1 unresolved discussion (waiting on @srfrog and @danielmai)

srfrog
srfrog commented Oct 20, 2018
View reviewed changes
Copy link
Contributor Author

@srfrog srfrog left a comment

Choose a reason for hiding this comment

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

Reviewable status: 17 of 18 files reviewed, 1 unresolved discussion (waiting on @danielmai)

contrib/tlstest/Makefile, line 58 at r1 (raw file):

Previously, danielmai (Daniel Mai) wrote…

"reload"

Done.

@danielmai
Copy link
Contributor

How do you run the tests? I ran make test and test3 failed, but it doesn't say why it failed.

$ make test
Generating CA cert in 'tls' dir.
make[1]: Entering directory '/home/dmai/go/src/github.com/dgraph-io/dgraph/dgraph'
make[1]: Leaving directory '/home/dmai/go/src/github.com/dgraph-io/dgraph/dgraph'
Test 1: Alpha non-TLS, Live non-TLS
Test 2: Alpha non-TLS, Live TLS
Test 3: Alpha TLS, Live non-TLS
./alpha_tls.sh <-> ./live_notls.sh: 0 != 1
Makefile:46: recipe for target 'test3' failed
make: *** [test3] Error 1

@srfrog
Copy link
Contributor Author

srfrog commented Oct 22, 2018

How do you run the tests? I ran make test and test3 failed, but it doesn't say why it failed.

make, make all or make test do all the same.

danielmai
danielmai approved these changes Oct 22, 2018
View reviewed changes
Copy link
Contributor

@danielmai danielmai left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 18 files at r1, 2 of 2 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @danielmai)

@srfrog srfrog merged commit 8f56eb9 into master Oct 22, 2018
@srfrog srfrog deleted the feature/issue-2615_new_tls_tests branch October 22, 2018 23:46
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 19, 2019
@srfrog @dna2github
Refactor tls test for new cert command. (hypermodeinc#2676)
b7bd0d5 
* Refactor tls test for new cert command.

Closes hypermodeinc#2615

* forgot to add test6 and clean

* fix typo in 'reload'

* removed vars from old tests

* explicit failure reason

* send KILL instead of INT to make sure all dgraphs are killed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielmai danielmai danielmai approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@srfrog @danielmai