Automate prism service. #229

dmitraver · 2017-10-04T05:39:25Z

Fixes #159.

-Installs prism on travis.
-Creates new gradle task to download and start prism locally. Prism is only downloaded if it wasn't downloaded before. To start prism locally do:

./gradlew startPrism

dmitry-avershin · 2017-10-04T07:21:55Z

@thinkingserious hi, should the build succeed now with my change or do you need to setup something on Travis first? 🤔

andy-trimble · 2017-10-09T23:55:35Z

test/prism.sh

+fi
+}
+
+install


Please add a new line at the end of the file.

andy-trimble · 2017-10-09T23:55:54Z

test/prism.sh

@@ -0,0 +1,42 @@
+#!/bin/bash


The Travis builds are currently failing because this file does not have execute permissions.

hmm, might be, I haven't seen chmod in any other projects travis.yaml files though.

I'm not suggesting this be done via Travis. Try the following and commit the changes:
chmod a+x test/prism.sh
Note that this won't work if you're on a Windows box. If that's the case, then I can make the change.

I think it doesn’t matter as executable flag is stored in inode and it’s not a part of the file.

Correct. This particular issue is resolved with a21f3da

@andy-trimble I still don't understand how this could solve the problem, permissions are stored in inode which is a structure that is stored somewhere on OS. chmod updates the inode. Could you plz explain where my understanding is wrong?

@dmitraver It is no longer a problem. You fixed it with commit a21f3da.

@andy-trimble yeah but for me this not enough, I want to figure out why its fixed behind the curtains. Just curious ;)

@dmitraver At the time that I made this comment, the Travis build was failing with the following message /home/travis/.travis/job_stages: line 57: ./test/prism.sh: Permission denied This indicates that the execute permission on the file were not present. Github preserves file permissions, and when Travis checked it out to execute the build, it was not executable.

@andy-trimble yup, I figured out that git actually preserves the file permissions .
git cat-file -p $hash permission bits are stored there as well.

andy-trimble · 2017-10-09T23:56:07Z

scripts/startPrism.sh

+   echo "Prism is not installed."
+   install
+   run
+fi


Please add a new line at the end of this file.

thinkingserious · 2017-10-10T00:17:12Z

Thanks for the review @andy-trimble!

dmitraver · 2017-10-10T13:52:57Z

@andy-trimble, fixed the issues you mentioned.

andy-trimble · 2017-10-10T15:27:40Z

@dmitraver The Travis build is still failing because Prism is not started up before the tests are run.

dmitraver · 2017-10-10T15:53:41Z

Hmm, I think before_script in travis.yaml does exactly that, maybe adding a sleep should help

andy-trimble · 2017-10-10T15:59:35Z

@dmitraver The before_script only installs Prism. It does not start the process.

dmitraver · 2017-10-10T20:35:03Z

@andy-trimble sure, you are right, my original understanding was that it is run by some other mean but I guess I'm wrong. I will update the scripts tomorrow.

dmitraver · 2017-10-11T15:27:41Z

@andy-trimble, @thinkingserious I fixed the prism startup. Build is still failing because of the issue Andy mentioned.

dmitraver has made the updates

thinkingserious · 2017-10-11T15:30:29Z

Which issue @dmitraver? It seems like you fixed the issues Andy mentioned.

dmitraver · 2017-10-11T15:38:20Z

@thinkingserious sorry, it was in his branch
b9647e4.

Builds are still failing due to IOExceptions on travis.

andy-trimble · 2017-10-11T15:44:16Z

@dmitraver The current exceptions in Travis are due to Prism not running. It seems that Prism is installed and started, but the process is killed before continuing on to the testing portion of the build.

thinkingserious · 2017-10-11T15:44:48Z

Can you add that to travis.yml please?

thinkingserious · 2017-10-11T16:29:36Z

@dmitraver, maybe try not running prism in the background.

andy-trimble · 2017-10-11T23:58:37Z

@dmitraver In my PR here, I'm using Groovy to kick off the Prism process, thus avoiding having Gradle shut down the process before the tests are run. Additionally, it waits until the Prism port is attached so that we don't have a race condition. Feel free to take whatever you need from that PR to keep your change moving forward. I really only made it as a reference implementation. I'm happy for you to get the Hacktoberfest credit, I'm already getting my swag. 😄

dmitraver · 2017-10-12T15:05:34Z

@thinkingserious running prism not in background seems to hang the process. I will try to experiment with it a bit. Locally it works fine.

dmitraver · 2017-10-15T09:25:05Z

hi @thinkingserious, @andy-trimble I've done some testing locally and found why the tests are failing on Jenkins. Most of the tests have this check

sendgrid-java/src/test/java/com/sendgrid/SendGridTest.java

Line 81 in 7cff48d

    
           if(System.getenv("TRAVIS") != null && Boolean.parseBoolean(System.getenv("TRAVIS"))) {

which resolves to the first branch on Travis because the TRAVIS env variable is set to 'true'. MOCK_HOST variable is not set on Travis so it basically uses empty string as a host and fails. I've tried to set the variable in Travis to localhost but it tries to use SSL then cause it assumes HTTPS. I think its because this branch is not intended to be used for local testing. So there are two solutions to this problem, first is to provide a real host with https connection for testing and set MOCK_HOST var in Travis for it. Second one is to remove this logic and only use localhost for testing. Let me know what is the behaviour you want and I will adapt the code.

andy-trimble · 2017-10-19T16:45:24Z

@dmitraver The TRAVIS environment variable isn't actually being set in Travis. You can verify this in the "Setting environment variables from .travis.yml" section in the Travis log.

Also, MOCK_HOST is being set to be localhost:4010 (as you can see in the Travis logs).

andy-trimble · 2017-10-19T17:11:13Z

@dmitraver One thing you could try is to insert a ps -ef | grep prism into the before_script section, after the sleep command. This should tell you if prism is actually running.

dmitraver · 2017-10-20T06:30:21Z

@andy-trimble hmm, this is actually strange cause I used their docker environment for testing and variable resolves to true there. The prism is running, you can see it from the output in travis. I also added the lsof command that checks that its running on port as intended. I still believe its due to the issue I pointed above as the docker environment is exactly the same as the real environment, the only thing that is different is that you need to run all your before scripts yourself. I can print the env variables.

dmitraver · 2017-10-20T06:33:36Z

@andy-trimble also I tried to set mock host variable in the last change and I started getting different exceptions on travis which is now that it can't connect with https connection to localhost so I guess this variable is not set at all.

dmitraver · 2017-10-20T06:41:12Z

@andy-trimble plz check the output of my last change https://travis-ci.org/sendgrid/sendgrid-java/jobs/290320581.

0.00s$ echo TRAVIS $TRAVIS
TRAVIS true
before_script.2
0.00s$ echo MOCK_HOST $MOCK_HOST
MOCK_HOST

so the travis variable is set but mock host is not.

dmitraver · 2017-10-20T09:07:04Z

@andy-trimble btw, here is the list of default env variables set in travis https://docs.travis-ci.com/user/environment-variables/#Default-Environment-Variables

…

On 19 October 2017 at 19:11, Andy Trimble ***@***.***> wrote: @dmitraver <https://github.com/dmitraver> One thing you could try is to insert a ps -ef | grep prism into the before_script section, after the sleep command. This should tell you if prism is actually running. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#229 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABtwDH8r0qa2hlT9mHAyderaGFVBNWsbks5st4K0gaJpZM4PtHnd> .

-- Best wishes, Dmitry Avershin.

dmitraver · 2017-10-20T14:19:30Z

@thinkingserious hi, how are we going to proceed with this one? I think I would remove this logic that checks if the test is running on travis and simply leave the branch which uses localhost there.

andy-trimble · 2017-10-20T15:14:50Z

Ah, well there you go @dmitraver 😄 . Good sleuthing. It's interesting that the MOCK_HOST isn't set. The output in Travis would indicated that it is being set. This is in the "Build system information" section of the output. I'm unsure how Travis works here.

From working on my PR, I noticed that I was having trouble keeping the Prism process running through the various Gradle steps. That's why I suggested checking for the process. I'm not seeing the output of the lsof command. Am I missing it?

andy-trimble · 2017-10-20T15:15:53Z

Also, @dmitraver, in order to validate your proposed solution, you could remove the logic from one of the tests, and see if that test passes in Travis.

dmitraver · 2017-10-20T15:17:46Z

@andy-trimble I think you've seen it there cause I added it before to test ;)
See c866a49

I will remove this logic and push the code to see if it actually builds.

dmitraver · 2017-10-20T19:46:27Z

@andy-trimble I've commented this Travis part from the 10 tests and these tests passed fine on Travis. I'm gonna remove it from all the tests then.

andy-trimble · 2017-10-20T23:26:42Z

Awesome! Sounds good @dmitraver.

dmitraver · 2017-10-21T06:14:06Z

@andy-trimble, @thinkingserious build is passing now.

mbernier · 2017-10-21T20:05:32Z

Wowza, I just caught up on this thread... Thank you so much @dmitraver and @andy-trimble - this was awesome.

mbernier · 2017-10-21T20:06:58Z

@andy-trimble if you approve your review, then we will get this merged in. I believe (and hope) it will fix the other build issues we're seeing too :)

andy-trimble · 2017-10-21T20:29:01Z

LGTM

thinkingserious · 2017-10-22T01:33:17Z

Hello @dmitraver,

Thanks again for the PR!

It's HACKTOBERFEST! We want to show our appreciation by sending you some special Hacktoberfest swag. If you have not already, could you please fill out this form so we can send it to you? Thanks!

Team SendGrid DX

dmitraver · 2017-10-22T12:49:59Z

@thinkingserious, @andy-trimble thx, I really appreciate that, happy to help ;)

dmitraver added 2 commits October 3, 2017 21:11

Adds prism installation script to travis.

2199f12

Creates gradle task to download and start prism locally.

008b04a

thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 4, 2017

thinkingserious mentioned this pull request Oct 9, 2017

Automate prism service on test #238

Closed

andy-trimble previously requested changes Oct 9, 2017

View reviewed changes

Adds new lines to the end of the files.

083f524

Makes prism script executable.

a21f3da

thinkingserious added difficulty: hard fix is hard in difficulty hacktoberfest labels Oct 10, 2017

Fixes prism startup script. Attempts to fix travis build.

665b0b4

Update .travis.yml

8895f22

Update .travis.yml

02c6f4e

Checks if the prism port is in use.

a43392f

Reverts back to running prism in background.

9b3f92c

dmitraver added 2 commits October 15, 2017 11:04

Reverts debugging statements.

1ae3ed2

Sets MOCK_HOST env variable in travis.

c866a49

Prints TRVIS and MOCK_HOST env variables.

25fafa8

Removes TRAVIS related logic from 10 tests.

ff020fa

Removes TRAVIS related logic from all tests.

802f36d

mbernier added difficulty: very hard fix is very hard in difficulty and removed difficulty: hard fix is hard in difficulty labels Oct 21, 2017

andy-trimble approved these changes Oct 21, 2017

View reviewed changes

Update .travis.yml

ff37ee0

thinkingserious merged commit 37dabf0 into sendgrid:master Oct 22, 2017

thinkingserious added the hacktoberfest label Oct 22, 2017

+              fi
+              }
+              install

Automate prism service. #229

Automate prism service. #229

Conversation

dmitraver commented Oct 4, 2017

dmitry-avershin commented Oct 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmitraver Oct 11, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thinkingserious commented Oct 10, 2017

dmitraver commented Oct 10, 2017

andy-trimble commented Oct 10, 2017

dmitraver commented Oct 10, 2017

andy-trimble commented Oct 10, 2017

dmitraver commented Oct 10, 2017

dmitraver commented Oct 11, 2017

thinkingserious commented Oct 11, 2017

dmitraver commented Oct 11, 2017

andy-trimble commented Oct 11, 2017

thinkingserious commented Oct 11, 2017

thinkingserious commented Oct 11, 2017

andy-trimble commented Oct 11, 2017 • edited Loading

dmitraver commented Oct 12, 2017

dmitraver commented Oct 15, 2017 • edited Loading

andy-trimble commented Oct 19, 2017

andy-trimble commented Oct 19, 2017

dmitraver commented Oct 20, 2017

dmitraver commented Oct 20, 2017

dmitraver commented Oct 20, 2017

dmitraver commented Oct 20, 2017 via email

dmitraver commented Oct 20, 2017

andy-trimble commented Oct 20, 2017

andy-trimble commented Oct 20, 2017

dmitraver commented Oct 20, 2017

dmitraver commented Oct 20, 2017

andy-trimble commented Oct 20, 2017

dmitraver commented Oct 21, 2017

mbernier commented Oct 21, 2017

mbernier commented Oct 21, 2017

andy-trimble commented Oct 21, 2017

thinkingserious commented Oct 22, 2017

dmitraver commented Oct 22, 2017

dmitraver Oct 11, 2017 •

edited

Loading

andy-trimble commented Oct 11, 2017 •

edited

Loading

dmitraver commented Oct 15, 2017 •

edited

Loading