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

Allow specifying make arguments for testing #109

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

henningkayser
Copy link
Member

@henningkayser henningkayser commented Apr 13, 2020

I ran into an issue where several integration tests using Gazebo caused a memory conflict resulting in the test runs being blocked and to fail. By limiting the number of make jobs to 1 (using -j1 inspired by ros-industrial/industrial_ci#446) all tests succeeded. I kept the new variable name generic, since I'm sure there are many more use cases for custom test options.

@mamoll
Copy link

mamoll commented Apr 13, 2020

Would it be possible to achieve the same thing by setting the environment variable MAKEFLAGS to -j1?

@henningkayser
Copy link
Member Author

Would it be possible to achieve the same thing by setting the environment variable MAKEFLAGS to -j1?

Probably, but then you would apply it for all build steps

Copy link

@v4hn v4hn left a comment

Choose a reason for hiding this comment

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

No harm in having the option to do that.

Still, parallel testing should be possible!
The issue you reference was about some state machine code, so I guess you experience different errors?
Could you at least show an error message?
If you run multiple gazebo tests in parallel you probably have to ensure individual server URIs for the tests to keep nodes from talking to the wrong server?

@rhaschke
Copy link
Contributor

To stay with the existing variable naming for tests, I suggest renaming the new parameter to TEST_MAKEFLAGS. I agree with Michael that you should actually try to fix your broken tests.
It should be possible to run multiple gazebo instances in parallel, isn't it?
Of course, you might run into memory limitations. Is that your actual problem?

@henningkayser
Copy link
Member Author

No harm in having the option to do that.

Still, parallel testing should be possible!
The issue you reference was about some state machine code, so I guess you experience different errors?
Could you at least show an error message?

Wasn't showing any errors, neither when running on Travis nor locally.

If you run multiple gazebo tests in parallel you probably have to ensure individual server URIs for the tests to keep nodes from talking to the wrong server?

Thanks, I didn't think about this.

To stay with the existing variable naming for tests, I suggest renaming the new parameter to TEST_MAKEFLAGS. I agree with Michael that you should actually try to fix your broken tests.

Will do, fixed the tests also

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.

4 participants