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

run tests from makefile #244

Merged
merged 14 commits into from
Nov 1, 2017
Merged

run tests from makefile #244

merged 14 commits into from
Nov 1, 2017

Conversation

vixentael
Copy link
Contributor

closes #243

check command

  • soter_test
  • themis_test
  • themispp_test

This is required test suit.

check_all command

  • phpthemis_test
  • pythemis_test
  • rubythemis_test
  • node

These tests require external dependencies, so they might fail due to outdated local configuration.

@vixentael vixentael added the infrastructure Automated building and packaging label Oct 30, 2017
@vixentael vixentael assigned vixentael and unassigned Lagovas Oct 30, 2017
@Lagovas
Copy link
Collaborator

Lagovas commented Oct 31, 2017

maybe we should refactor this makefile and make that make test will run tests (not compile them)? What about to rename test target to prepare_test (or something like this) and new check target to test? after that make test will do what expected - test current code

and I would use this updated Makefile in our circle.yml

@vixentael
Copy link
Contributor Author

Nice idea! But then we should update Makefiles for other products to be consistent :)

I'll do it

@vixentael
Copy link
Contributor Author

vixentael commented Oct 31, 2017

@Lagovas

compile tests:

  • prepare_tests_basic
  • prepare_tests_all

run tests:

  • test_basic
  • test

by default, make test will prepare all tests and run them

If it's ok for you, I'll update wiki

Makefile Outdated
@@ -567,7 +526,7 @@ symlink_realname_to_soname:
strip:
@find . -name \*.$(SHARED_EXT)\.* -exec strip -o {} {} \;

deb: test soter_static themis_static soter_shared themis_shared collect_headers install_shell_scripts strip symlink_realname_to_soname
deb: prepare_tests_basic soter_static themis_static soter_shared themis_shared collect_headers install_shell_scripts strip symlink_realname_to_soname
Copy link
Collaborator

Choose a reason for hiding this comment

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

prepare_tests_baisc -> test_basic. run tests before building package

Makefile Outdated
@@ -610,7 +569,7 @@ deb: test soter_static themis_static soter_shared themis_shared collect_headers
@find $(BIN_PATH) -name \*.deb


rpm: test themis_static themis_shared soter_static soter_shared collect_headers install_shell_scripts strip symlink_realname_to_soname
rpm: prepare_tests_basic themis_static themis_shared soter_static soter_shared collect_headers install_shell_scripts strip symlink_realname_to_soname
Copy link
Collaborator

Choose a reason for hiding this comment

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

prepare_tests_baisc -> test_basic. run tests before building package

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do it, but this PR won't be 'refactoring' anymore (because we're changing logic here, not just renaming)

Copy link
Collaborator

Choose a reason for hiding this comment

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

logic the same - test code before package building but before this logic was broken, but now you fix it and it's great

to finish this refactoring we should use new make test in our circle.yml instead of current commands:

...
    - build/tests/soter_test
    - build/tests/themis_test
    - build/tests/themispp_test
    - build/tests/pythemis_test.sh
    - build/tests/phpthemis_test.sh
    - build/tests/node.sh
...

what you think?

@vixentael
Copy link
Contributor Author

@Lagovas

  • circle.yml updated
  • wiki updated

@Lagovas
Copy link
Collaborator

Lagovas commented Oct 31, 2017

to finish this refactoring we should use new make test in our circle.yml instead of current commands:

...
- build/tests/soter_test
- build/tests/themis_test
- build/tests/themispp_test
- build/tests/pythemis_test.sh
- build/tests/phpthemis_test.sh
- build/tests/node.sh
...
what you think?

@vixentael
Copy link
Contributor Author

vixentael commented Oct 31, 2017

@Lagovas I think that we shouldn't use make test in circle.yml, because it's more useful to see what tests are failing.

However, I did more research & refactoring on that, and got rid of running ruby tests with sudo.

@Lagovas
Copy link
Collaborator

Lagovas commented Nov 1, 2017

@Lagovas I think that we shouldn't use make test in circle.yml, because it's more useful to see what tests are failing.

yeah, it's good when used public interface of Makefile (like you did now), not when used the implementation in recipes (as it was before).

@Lagovas Lagovas merged commit a9af822 into master Nov 1, 2017
@vixentael vixentael deleted the vixentael/run-tests branch November 1, 2017 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Automated building and packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Makefile to run tests
2 participants