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

Wrong entity_refs_ counter when deleting data reader in the TopicDescriptionImpl #936

Merged
merged 7 commits into from Oct 2, 2018

Conversation

Projects
None yet
4 participants
@jmmorato
Copy link
Contributor

commented Sep 23, 2018

Fixes #935

@jwillemsen

This comment has been minimized.

Copy link
Collaborator

commented Sep 23, 2018

Would be helpful when you can extend/add a unit test as reproducer to make sure it works and never gets broken again by accident

@jmmorato

This comment has been minimized.

Copy link
Contributor Author

commented Sep 23, 2018

In fact, the test already exists, at least for the content filtered topic tests\DCPS\ContentFilteredTopic (I got the idea for my unit test from there), but I guess it is not called during CI process.
For the MultiTopic could be easily extended in tests\DCPS\MultiTopic. For the Topic itself, I cannot see something similar on the test folder but perhaps you could give me an idea where is the best place to code it.

BTW, I having problems to configure --security --features=versioned_namespace=1 --no-inline (the configuration failing in the pull request) because a not found qt5_witges dependency. If someone could take a look on it would be great.

@iguessthislldo

This comment has been minimized.

Copy link
Member

commented Sep 23, 2018

BTW, I having problems to configure --security --features=versioned_namespace=1 --no-inline (the configuration failing in the pull request) because a not found qt5_witges dependency. If someone could take a look on it would be great.

Actually there two seperate known issues going on here:

  1. Currently we do not pay Travis, so we get to use Travis for free because OpenDDS is open source, but our time is limited to 50 minutes, so this fails in at least one of the cases all the time. It would actually be surprising if everything finished up in 50 minutes and passed.

  2. There has been a recent switch from Qt4 to Qt5 and that change had to be made across MPC, ACE/TAO, and OpenDDS. Unfortunately these changes have not been incorporated into the versions of ACE/TAO/MPC that OpenDDS configure script downloads by default yet. For now It fails unless you download the latest MPC separately. It's not hard thing to fix though, you download MPC from https://github.com/DOCGroup/MPC and set the --mpc option to there when you run the configure script.

@mitza-oci

This comment has been minimized.

Copy link
Member

commented Sep 24, 2018

ok to test

This should start the Jenkins build on our server. When I have some more time available I can give it a full review and take a look at what broke and how to address it with test coverage.

iguessthislldo added a commit to iguessthislldo/OpenDDS that referenced this pull request Sep 25, 2018

Fix QC Test Failure
Disposed Sample Filter Tests are failing. Deleting the entities between
tests fixes this.

This relies on objectcomputing#936 as
delete_datareader fails at the moment. This test should work once it is
merged.
@mitza-oci

This comment has been minimized.

Copy link
Member

commented Sep 27, 2018

Now that #938 is merged, the required test coverage has been added.
In the Jenkins build for this PR, the Ownership test now fails with:

ERROR: <subscriber> exited with coredump from signal 11 : SEGV
ERROR: subscriber returned 255 
test FAILED.
Error: tests/DCPS/Ownership/run_test.pl returned with status 256

@mitza-oci mitza-oci merged commit eb9e483 into objectcomputing:master Oct 2, 2018

4 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
OpenDDS_Pull_Requests Build #337 (merge) stopped
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

mitza-oci pushed a commit that referenced this pull request Nov 12, 2018

Fix QC Test Failure
Disposed Sample Filter Tests are failing. Deleting the entities between
tests fixes this.

This relies on #936 as
delete_datareader fails at the moment. This test should work once it is
merged.

mitza-oci added a commit that referenced this pull request Nov 12, 2018

Merge pull request #936 from jmmorato/master
 Wrong entity_refs_ counter when deleting data reader in the TopicDescriptionImpl

mitza-oci added a commit that referenced this pull request Apr 1, 2019

Merge pull request #936 from jmmorato/master
 Wrong entity_refs_ counter when deleting data reader in the TopicDescriptionImpl

(cherry picked from commit eb9e483)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.