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

Valgrind supppressions #9633

Closed

Conversation

donaldsharp
Copy link
Member

see individual commits

… time

The tests where concatenating the string to run particular daemons together
without a space causing the whole thing to not work.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
When running topotests w/ valgrind and we call the
check_for_memleaks() function call, tests were aborting
because valgrind was finding a bunch of memory leaks.
The whole point of valgrind is to not stop the tests
when we find a memory leak.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
@frrbot frrbot bot added the tests Topotests, make check, etc label Sep 16, 2021
@LabN-CI
Copy link
Collaborator

LabN-CI commented Sep 16, 2021

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table
_ _
Result SUCCESS git merge/9633 3475637
Date 09/16/2021
Start 17:40:53
Finish 18:07:15
Run-Time 26:22
Total 1813
Pass 1813
Fail 0
Valgrind-Errors 0
Valgrind-Loss 0
Details vncregress-2021-09-16-17:40:53.txt
Log autoscript-2021-09-16-17:42:06.log.bz2
Memory 488 517 426

For details, please contact louberger

@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Sep 17, 2021

Continuous Integration Result: FAILED

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-227/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests debian 10 amd64 part 5: Failed (click for details) Topotests debian 10 amd64 part 5: No useful log found
Successful on other platforms/tests
  • Topotests debian 10 amd64 part 9
  • Addresssanitizer topotests part 7
  • Topotests Ubuntu 18.04 arm8 part 8
  • Topotests Ubuntu 18.04 i386 part 2
  • Topotests Ubuntu 18.04 i386 part 7
  • Topotests Ubuntu 18.04 amd64 part 3
  • Topotests Ubuntu 18.04 amd64 part 2
  • Addresssanitizer topotests part 5
  • Topotests Ubuntu 18.04 i386 part 6
  • IPv4 ldp protocol on Ubuntu 18.04
  • Ubuntu 16.04 deb pkg check
  • Topotests Ubuntu 18.04 amd64 part 4
  • Topotests Ubuntu 18.04 i386 part 8
  • Topotests Ubuntu 18.04 i386 part 1
  • Topotests debian 10 amd64 part 8
  • Topotests Ubuntu 18.04 arm8 part 9
  • Topotests Ubuntu 18.04 arm8 part 4
  • Addresssanitizer topotests part 2
  • Ubuntu 20.04 deb pkg check
  • IPv6 protocols on Ubuntu 18.04
  • Topotests Ubuntu 18.04 amd64 part 0
  • Topotests Ubuntu 18.04 amd64 part 5
  • Debian 10 deb pkg check
  • Fedora 29 rpm pkg check
  • IPv4 protocols on Ubuntu 18.04
  • Topotests Ubuntu 18.04 amd64 part 7
  • Topotests Ubuntu 18.04 arm8 part 2
  • Topotests debian 10 amd64 part 0
  • Topotests Ubuntu 18.04 arm8 part 7
  • Addresssanitizer topotests part 3
  • Topotests Ubuntu 18.04 i386 part 3
  • Addresssanitizer topotests part 9
  • Addresssanitizer topotests part 8
  • Topotests Ubuntu 18.04 arm8 part 5
  • CentOS 7 rpm pkg check
  • Topotests Ubuntu 18.04 amd64 part 9
  • Addresssanitizer topotests part 6
  • Static analyzer (clang)
  • Topotests debian 10 amd64 part 1
  • Topotests Ubuntu 18.04 i386 part 5
  • Topotests Ubuntu 18.04 amd64 part 1
  • Topotests Ubuntu 18.04 i386 part 0
  • Topotests Ubuntu 18.04 amd64 part 8
  • Topotests debian 10 amd64 part 2
  • Topotests debian 10 amd64 part 7
  • Topotests Ubuntu 18.04 arm8 part 0
  • Topotests Ubuntu 18.04 amd64 part 6
  • Topotests debian 10 amd64 part 3
  • Addresssanitizer topotests part 0
  • Ubuntu 18.04 deb pkg check
  • Topotests Ubuntu 18.04 arm8 part 6
  • Topotests Ubuntu 18.04 arm8 part 1
  • Topotests debian 10 amd64 part 6
  • Addresssanitizer topotests part 1
  • Topotests Ubuntu 18.04 arm8 part 3
  • Topotests Ubuntu 18.04 i386 part 4
  • Topotests Ubuntu 18.04 i386 part 9
  • Topotests debian 10 amd64 part 4
  • Addresssanitizer topotests part 4
  • Debian 9 deb pkg check

@donaldsharp
Copy link
Member Author

ci:rerun

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-231/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Copy link
Contributor

@choppsv1 choppsv1 left a comment

Choose a reason for hiding this comment

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

The intent of the code as written was to report and fail any test where valgrind finds errors. This is only triggered when the tests are run with the valgrind, and this only happens when the option being discussed here is specified, so the user is looking for these errors. I do not agree that the point of valgrind here is to ignore memleaks.

@Jafaral Jafaral added this to the 8.1 milestone Sep 21, 2021
@donaldsharp
Copy link
Member Author

Running valgrind against all tests is impossible currently without the second commit. As that it will error out after the first test. This is fundamentally broken and must be fixed. If this is not the right approach then please give us a PR where the right approach is used.

@Jafaral
Copy link
Member

Jafaral commented Oct 5, 2021

Superseded by #9633

@donaldsharp donaldsharp closed this Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Topotests, make check, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants