Skip to content
This repository was archived by the owner on Oct 21, 2024. It is now read-only.

Profiler test udp connection#51

Open
baselqyqony wants to merge 12 commits intomasterfrom
profiler_test_udp_connection
Open

Profiler test udp connection#51
baselqyqony wants to merge 12 commits intomasterfrom
profiler_test_udp_connection

Conversation

@baselqyqony
Copy link
Copy Markdown
Collaborator

  • add TestUDPUsingSchedular test to ProfilerTest

Copy link
Copy Markdown
Owner

@gerazo gerazo left a comment

Choose a reason for hiding this comment

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

This tricky failure needs testing. It can happen that you run 100 times and only once it fails. For this, you can write a simple script.

#include <cstdio>
#include <thread>

#include "gtest/gtest.h"
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Putting the test headers in front of anything else does make sense in this case.

!packet_to_process.IsEmpty(); packet_to_process = pc.Poll()) {
}
// test receiving data
EXPECT_EQ(Receive(), true);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This line fails in clang release (returns false). It would be good to find out, why.

@gerazo
Copy link
Copy Markdown
Owner

gerazo commented Jul 16, 2021

The last one could trigger the same problem also in Debug. This means that clang, the compiler does something different which causes this problem.

@baselqyqony
Copy link
Copy Markdown
Collaborator Author

baselqyqony commented Jul 19, 2021

I found the problem root cause, in the release on clang phase the buffer has different values from the expected ones, because of that the function receives returns false

@gerazo
Copy link
Copy Markdown
Owner

gerazo commented Jul 20, 2021

Thank @baselqyqony nice catch. I will look into it. I am creating an issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants