Script to setup integration tests #293
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
piotrkwiecinski
force-pushed
the
setup-integration-tests
branch
from
b1ed58d
to
62010da
Compare
piotrkwiecinski
commented
Aug 8, 2020
compose/bin/setup-integration-tests
Outdated
| REAL_SRC=$(cd -P "src" && pwd) | ||
| MYSQL_CONFIG=dev/tests/integration/etc/install-config-mysql.php | ||
| INTEGRATION_DB=magento_integration_tests | ||
| CREATE_DB="bin/clinotty mysqladmin -hdb -uroot -p${MYSQL_ROOT_PASSWORD} create ${INTEGRATION_DB}" && echo "Database ${INTEGRATION_DB} created." |
There was a problem hiding this comment.
I couldn't find cleaner way to handle exit code from command run on a container. Suggestions are welcome.
There was a problem hiding this comment.
would this line be better?
CREATE_DB="bin/clinotty mysqladmin -hdb -uroot -p${MYSQL_ROOT_PASSWORD} create ${INTEGRATION_DB}" || exit
piotrkwiecinski
commented
Aug 8, 2020
compose/bin/setup-integration-tests
Outdated
| CREATE_DB="bin/clinotty mysqladmin -hdb -uroot -p${MYSQL_ROOT_PASSWORD} create ${INTEGRATION_DB}" && echo "Database ${INTEGRATION_DB} created." | ||
| bin/clinotty mysql -hdb -uroot -p${MYSQL_ROOT_PASSWORD} ${INTEGRATION_DB} -e exit &> /dev/null || $CREATE_DB | ||
|
|
||
| sed -e "s/'db-host' => 'localhost'/'db-host' => 'db'/" \ |
There was a problem hiding this comment.
Are we ok with using root user or do we want to create a separate user?
We could put credentials in env/db.env if you prefer.
There was a problem hiding this comment.
I think root user is fine, as most of these scripts are meant for development. However, I think putting creds in env/db.env are the right way to go:
...
MYSQL_INTEGRATION_ROOT_PASSWORD=magento
MYSQL_INTEGRATION_DATABASE=magento_integration
MYSQL_INTEGRATION_USER=magento
MYSQL_INTEGRATION_PASSWORD=magento
etc
Note renaming INTEGRATION_DB to MYSQL_INTEGRATION_DATABASE and moving that into the env.db file. I'd also keep all the users/creds/etc magento for simplicity sake.
piotrkwiecinski
commented
Aug 8, 2020
compose/bin/setup-integration-tests
Outdated
| src/dev/tests/integration/etc/install-config-mysql.php.dist > src/dev/tests/integration/etc/install-config-mysql.php | ||
|
|
||
| # Workaround until coping nested files works as expected | ||
| docker cp $REAL_SRC/$MYSQL_CONFIG $(docker-compose ps -q phpfpm|awk '{print $1}'):/var/www/html/$MYSQL_CONFIG |
There was a problem hiding this comment.
This can be simplified after #295 is merged.
markshust
requested changes
Aug 27, 2020
compose/bin/setup-integration-tests
Outdated
| REAL_SRC=$(cd -P "src" && pwd) | ||
| MYSQL_CONFIG=dev/tests/integration/etc/install-config-mysql.php | ||
| INTEGRATION_DB=magento_integration_tests | ||
| CREATE_DB="bin/clinotty mysqladmin -hdb -uroot -p${MYSQL_ROOT_PASSWORD} create ${INTEGRATION_DB}" && echo "Database ${INTEGRATION_DB} created." |
There was a problem hiding this comment.
would this line be better?
CREATE_DB="bin/clinotty mysqladmin -hdb -uroot -p${MYSQL_ROOT_PASSWORD} create ${INTEGRATION_DB}" || exit
compose/bin/setup-integration-tests
Outdated
| CREATE_DB="bin/clinotty mysqladmin -hdb -uroot -p${MYSQL_ROOT_PASSWORD} create ${INTEGRATION_DB}" && echo "Database ${INTEGRATION_DB} created." | ||
| bin/clinotty mysql -hdb -uroot -p${MYSQL_ROOT_PASSWORD} ${INTEGRATION_DB} -e exit &> /dev/null || $CREATE_DB | ||
|
|
||
| sed -e "s/'db-host' => 'localhost'/'db-host' => 'db'/" \ |
There was a problem hiding this comment.
I think root user is fine, as most of these scripts are meant for development. However, I think putting creds in env/db.env are the right way to go:
...
MYSQL_INTEGRATION_ROOT_PASSWORD=magento
MYSQL_INTEGRATION_DATABASE=magento_integration
MYSQL_INTEGRATION_USER=magento
MYSQL_INTEGRATION_PASSWORD=magento
etc
Note renaming INTEGRATION_DB to MYSQL_INTEGRATION_DATABASE and moving that into the env.db file. I'd also keep all the users/creds/etc magento for simplicity sake.
Update command to handle db creation and permissions
piotrkwiecinski
force-pushed
the
setup-integration-tests
branch
from
62010da
to
db0bb72
Compare
piotrkwiecinski
changed the title
|
@markshust it should be ready for another round. Credentials are used from env/db.env. If everything goes as expected you should be able to run |
markshust
approved these changes
Oct 8, 2020
|
@piotrkwiecinski I merged in because there aren't any breaking updates here, but I got this error when running it: |
|
Later in the error I also see this... |
|
@markshust tasklist exec should be resolved by: 1b71f7f but I'm not sure if you update an image after that change was merged. I'll have a look a the amqp tomorrow or on saturday. That branch is a bit old :). |
|
Ah thanks -- I'll test with the new image build |
|
Just for reference, here's output with the new PHP image: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Script to setup env for integration tests
It creates a integration database and generates
install-config-mysql.phpbased on the .dist version.