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

Add script to test TiSpark #33

Merged
merged 5 commits into from Jul 25, 2018
Merged

Conversation

liufuyang
Copy link
Contributor

@liufuyang liufuyang commented Jul 23, 2018

To add another test to verify the tispark component can work.

So to have some early warning of errors mentioned here:
#30

Current build job will fail due to issue mentioned at #30

@tennix Do you mind help re-run the test of this PR when the TiSpark docker image get updated?

@liufuyang
Copy link
Contributor Author

liufuyang commented Jul 23, 2018

image

BTW: Pretty sad that there is only a tag on tispark docker image, making me cannot roll back to an old tispark image version locally to test the tests.py script will actually work 😞

.travis.yml Outdated
@@ -12,3 +12,5 @@ before_install:
script:
- docker ps -a --format="{{.Names}} {{.Image}} {{.Status}}" | grep -v 'Up' | grep -v 'Exited (0)' | awk '{print} END {if (NR>0) {exit 1;}}'
- mysql -h 127.0.0.1 -P 4000 -u root -e "select tidb_version()\G" # test if tidb-server is working
- docker-compose exec tispark-master bash /opt/tispark/tests/loaddata.sh # add some data for tests
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to load data from host.

  1. download and extract tispark-sample-data
  2. load data from host
    These two steps can be done in before_install section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we do that (or why)? those data already extracted in the master image/container.
This call only inject the data into tidb my mysql interface. There is no downloading.

Copy link
Member

Choose a reason for hiding this comment

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

If we can load data from host, we are sure we can access TiDB, so no need to do the select tidb_version() step. And we don't need to add extra script to tispark-master container. But if you insist your option, I'm OK with it.

@@ -123,6 +123,7 @@ services:
- /opt/spark/sbin/start-master.sh
volumes:
- ./config/spark-defaults.conf:/opt/spark/conf/spark-defaults.conf:ro
- ./tispark/tests:/opt/tispark/tests:ro
Copy link
Member

Choose a reason for hiding this comment

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

We can start another container connecting to tidb-docker-compose network and run tests there without affecting tispark-master container.

networks:
default:
external:
name: tidbdockercompose_default # when travis'd docker version updates, use: tidb-docker-compose_default
Copy link
Member

Choose a reason for hiding this comment

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

You can upgrade travis docker and docker-compose version as needed. https://docs.travis-ci.com/user/docker/#Installing-a-newer-Docker-version

@liufuyang
Copy link
Contributor Author

@tennix finally it's working now. I had to update the docker-compose version as well to make it work with the newer way of network name.
Also I think it is nice to have some db data in the tispark-master. It's pretty convenient for people to start containers from this repo then run an command to inject data. Unless we are going to remove it from another PR, then we can update the script to download the data from internet when doing the tests.

@liufuyang
Copy link
Contributor Author

And let me know if you want me to squash everything into a single commit.

Otherwise I think you can use the squash and merge option on the merge button.

Looks like the tispark issue has been fixed, the build can all go through now.

Copy link
Member

@tennix tennix left a comment

Choose a reason for hiding this comment

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

LGTM

@tennix
Copy link
Member

tennix commented Jul 25, 2018

Adding another docker-compose file for testing looks good.

@tennix tennix merged commit f30d7af into pingcap:master Jul 25, 2018
@liufuyang liufuyang deleted the add-more-tests branch July 25, 2018 05:24
tennix pushed a commit that referenced this pull request Aug 15, 2018
* Add script to test TiSpark

* Remove mysql test, use tispark-tests to test tidb connection
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