-
Notifications
You must be signed in to change notification settings - Fork 536
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
Remove hardcoded endpoint #4657
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4657 +/- ##
==========================================
- Coverage 87.27% 86.45% -0.82%
==========================================
Files 56 56
Lines 17361 17361
==========================================
- Hits 15152 15010 -142
- Misses 2209 2351 +142 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
src/cs/tool/Program.cs
Outdated
@@ -32,6 +32,7 @@ static unsafe void Main(string[] args) | |||
} | |||
|
|||
var ApiTable = MsQuic.Open(); | |||
const string DomainName = "google.com"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you still hard code it here...? How does this actually fix it? My understanding of the problem is that you any URL in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this "tool" want to do?
Connection establishment over internet using C#?
Then any endpoint is needed, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, just a sanity check that it can use C# + msquic to make a connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the next step?
- Report ignoring?
- Remove this tool?
- Connect localhost?
- Is there any acceptable endpoint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just remove the tool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Crap. Looks like the tests were using this. Let's add back the tool, make the URL a command line arg, and then update the test code to pass in google.com instead.
c76c9e0
to
5bb2241
Compare
Description
Removed hardcoded endpoint ("google.com")
Reported by https://vnext.s360.msftcloudes.com/blades/quality/reliability?blade=AssignedTo:All~KPI:510f681d-1be2-4568-a686-85d1fc0e5006~SLA:0~Forums:All~KPI%20Ranking:All~waves:All~Tab:Summary~_loc:Reliability&peopleBasedNodes=nibanks_team&global=4:b7b51c2f-738e-43e4-8f76-05d9a8e5f6d7
Testing
CI
Documentation
N/A