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

Use ctest and install examples #37

Merged
merged 3 commits into from
Jul 14, 2012
Merged

Use ctest and install examples #37

merged 3 commits into from
Jul 14, 2012

Conversation

kensington
Copy link
Contributor

  1. Use ctest instead of a custom target
  2. Install examples if they are built

@@ -45,6 +45,12 @@ ADD_EXECUTABLE(strip-id3v1 strip-id3v1.cpp)

TARGET_LINK_LIBRARIES(strip-id3v1 tag )

INSTALL(TARGETS
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these should be installed.

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 note that this would only be called if BUILD_EXAMPLES is enabled.

How do you feel about an INSTALL_EXAMPLES target instead?

@lalinsky
Copy link
Member

And I have a few questions, I'm using make test to run the tests, but:

  • If a test fails, I don't see which test failed. Is there a way to display the stdout/stderr from the test runner?
  • It seems that make test doesn't depend on the test runner target, so that if I change the tests and run make test, it will not rebuild the sources. Is that expected?

@kensington
Copy link
Contributor Author

Thanks for the review.

The behaviour you have described is either a feature of a bug of ctest, depending on who you talk to. I have updated the pull request to restore the old check target with verbose output, while still making use of ctest.

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.

2 participants