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

Fix gz_test when using a CMake generator different from make #204

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

traversaro
Copy link
Contributor

@traversaro traversaro commented Oct 30, 2022

🦟 Bug fix

The PR fixes a failure when running test using CMake generators different from Unix Makefiles, such as ninja .

Summary

This fix permits to run the test suite without problems also when using the ninja generator.
I am not sure this make the test run for all possible CMake generators (for example, I guess it would fail for Multiple-Config Generators) but anyhow it is a strict improvement.

Without this fix, running the tests with ninja would fail with:

2022-10-30T19:16:15.7716940Z /Users/runner/mambaforge/conda-bld/gz-launch6_1667156961572/work/src/cmd/gz_TEST.cc:71: Failure
2022-10-30T19:16:15.7818910Z Value of: output.find("Makefile") != std::string::npos
2022-10-30T19:16:15.7920580Z   Actual: false
2022-10-30T19:16:15.8001560Z Expected: true
2022-10-30T19:16:15.8003040Z CMakeFiles
2022-10-30T19:16:15.8017710Z CTestTestfile.cmake
2022-10-30T19:16:15.8018820Z cmake_install.cmake
2022-10-30T19:16:15.8082140Z cmdlaunch6.rb
2022-10-30T19:16:15.8083300Z cmdlaunch6.rb.configured
2022-10-30T19:16:15.8084830Z launch6.bash_completion.sh
2022-10-30T19:16:15.8085850Z launch6.yaml
2022-10-30T19:16:15.8088570Z launch6.yaml.configured
2022-10-30T19:16:15.8089650Z test_cmdlaunch6.rb.configured

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@github-actions github-actions bot added the 🌱 garden Ignition Garden label Oct 30, 2022
@traversaro
Copy link
Contributor Author

(I am on an airport and I have some trouble using a proper terminal, so the DCO fix will have to wait for PST afternoon/evening).

@traversaro
Copy link
Contributor Author

Related downstream PR: conda-forge/staged-recipes#20749 .

@traversaro
Copy link
Contributor Author

(I am on an airport and I have some trouble using a proper terminal, so the DCO fix will have to wait for PST afternoon/evening).

Fixed!

@mjcarroll
Copy link
Contributor

Without this fix, running the tests with ninja would fail with:

Very likely not the only place. That is also the intention of using this: gazebosim/gz-cmake#301

@traversaro
Copy link
Contributor Author

@mjcarroll I may be asking something stupid, but how do I know the proper way of splitting the line to fit the 80 column limit?

@nkoenig
Copy link
Contributor

nkoenig commented Nov 1, 2022

@mjcarroll I may be asking something stupid, but how do I know the proper way of splitting the line to fit the 80 column limit?

There is no official rule on how to split a line. Generally, we try to split it at a logical point. In your case, you could move the << output;.

@traversaro
Copy link
Contributor Author

Thanks!

@nkoenig nkoenig merged commit 6bb69a0 into gazebosim:gz-launch6 Nov 2, 2022
nkoenig pushed a commit that referenced this pull request Nov 2, 2022
Signed-off-by: Nate Koenig <[email protected]>
nkoenig added a commit that referenced this pull request Nov 2, 2022
Signed-off-by: Nate Koenig <[email protected]>

Signed-off-by: Nate Koenig <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants