Fixed issue that prevented Postgres Tests from passing locally and on any port other than 5432 in travis

[cbaker6]

For postgres testers if you attempt to run a local test using the official Postgres (which PostGIS docker image uses), POSTGRES_PASSWORD has to be set or else the image won't build. This means that all local postgres tests using docker will require the use if PARSE_SERVER_TEST_DATABASE_URI as the default postgres URI set in the spec won't work.

Currently, two tests in the spec cause failure in this case, ParseQuery.FullTextSearch.spec.js and PostgresInitOptions.spec.js. This is because they have hardcoded URI's and locally create their own adaptors instead of using the Config (from what I've seen, tests that depend on the adapter typically use config). In the case of PostgresInitOptions.spec.js, it tests creating a new adaptor, so it makes since not to depend on config here, the problem is that it currently doesn't know about PARSE_SERVER_TEST_DATABASE_URI from the spec which lets you define your own URI for testing.

The reciprocal of this problem is running the test on travis and attempting to use the default port, 5433, that the image installs add-on versions of postgres on (standard port for postgres in general is 5432). Running travis on any port other than 5432 will cause ParseQuery.FullTextSearch.spec.js and PostgresInitOptions.spec.js to fail due to the aforementioned issues. The main branch gets around this by changing the config file for the postgres image to port 5432, but also needs to remove unnecessary postgres versions for it to run without issues.

[cbaker6]

This is ready to be reviewed. I expected the postgres time to improve a little more, but it looks like travis is currently having slower times than normal

[cbaker6]

I updated the contributing guide for running postgres tests. The rest of the updates for how to run in docker are in pull request #6506

[cbaker6]

On a related note and for future reference, I think the changes here helped me find the root cause of #6374. My assumption is that when previously running tests with postgres-9, the issue didn't show because postgres-9 is pre-installed on the image and runs on port 5432. The change to postgres 11 required an add-on and two issues were created:

1. Pre-installed versions of postgres run on port 5432
2. Using sudo service postgresql start ${ADD_ON_VERSION}

Looking at the raw logs of travis, when postgres installs as an add-on, it stops all postgres services and starts up the add-on version using: sudo systemctl start postgresql@${ADD_ON_VERSION}-main. It doesn't use "sudo service..." and my guess is there are some environment vars that aren't updated to point to the add-on version. This caused a conflict because something (I don't know what it was) was looking for postgis on the default port of 5432, but instead found the add-on version which had an updated version of postgis.

So if you really want to run on 5432, change the port in the add-on as before, sudo sed -i 's/port = 5433/port = 5432/' /etc/postgresql/${POSTGRES_MAJOR_VERSION}/main/postgresql.conf and then start the service using sudo systemctl start postgresql@${ADD_ON_VERSION}-main instead of sudo service postgresql start ${ADD_ON_VERSION}.

Or, with the fix in this branch that fixes the hard coded URIs in two of the spec files, simply use the already included PARSE_SERVER_TEST_DATABASE_URI to set the URI to the default add-on port of 5433, PARSE_SERVER_TEST_DATABASE_URI=postgres://localhost:5433/parse_server_postgres_adapter_test_database

[acinader]

@dplewis can we merge this one too?

[dplewis]

This looks good to me. We could probably do the same for mongo in the future.

I’ll run this locally right now then merge after

[dplewis]

Just a quick change from me and we should be good to go!

[dplewis]

Can you change the run test command to include npm run testonly so that it doesn't start the mongodb-runner

PARSE_SERVER_TEST_DB=postgres PARSE_SERVER_TEST_DATABASE_URI=postgres://postgres:password@localhost:5432/parse_server_postgres_adapter_test_database npm run testonly

[cbaker6]

Done

[dplewis]

@cbaker6 Can you run the testonly locally? Making sure you don't have mongo running locally? I have 6 failing tests that require mongodb

[cbaker6]

I can try, I've always ran "npm test" because of the older versions of the docs

[dplewis]

testonly was added because we didn't want to keep starting the mongodb-runner. It makes testing locally faster.

[dplewis]

In the future can you delete your fork, re create it and work off of branches instead of your master branch?

[cbaker6]

@dplewis thanks, I didn't know this. I will try it next time

[dplewis]

Even though we squash commits. As you can see there are 60+ commits that carried over from previous PRs that you have done. A rule of thumb is to keep the master branch on your fork up to date and branch off of that before every PR.

[acinader]

what i do is create a 'feature' branch that I put all my commits into and I periodically merge master into it.

making tons of small commits is a good practice imho (not disagreeing with dplewis, just adding my$.02).

[dplewis]

I have a develop branch that I use similar concept.

[dplewis]

LGTM! Thanks for the dedication to the project.