#4373, fix possible winreg value type difference

[mingyuan-xia]

No description provided.

[codecov-io]

Codecov Report

Merging #4377 into master will increase coverage by 0.68%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4377      +/-   ##
==========================================
+ Coverage   88.36%   89.05%   +0.68%     
==========================================
  Files          18       18              
  Lines        2055     2055              
==========================================
+ Hits         1816     1830      +14     
+ Misses        239      225      -14

Impacted Files	Coverage Δ
requests/utils.py	86.72% <100%> (+3.31%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e4fc353...7e1fb02. Read the comment docs.

[Lukasa]

Cool, looks plausible to me. Can you write some tests for each case? It's sufficient to monkeypatch QueryValueEx for these tests, as we never run on Windows.

[mingyuan-xia]

It seems that test_should_bypass_proxies_win_registry only monkeypatches WinReg but never tests anything (no assert~). Will adding another monkeypatch there be sufficient?

[Lukasa]

This is not sufficient, as it has never been used. However, you're right that this never actually tests anything, which isn't great. This was supposed to call should_bypass_proxies: want to add that call, and then parametrise this test based on the return value of QueryValueEx with ProxyEnable?

[mingyuan-xia]

Let me try adding some real assertions here. Need to find a Windows first, though.

[mingyuan-xia]

Since we already have multiple parameterized tests, I simply rotate the ProxyEnable Windows registry key type per test to simulate the case. Also I have added an assertion to test the proxy selection as it was supposed to be there.

[Lukasa]

Looks like the tests are failing: mind addressing the test failures?

[sigmavirus24]

Hi @mingyuan-xia the tests are passing but the branch isn't up-to-date with master. If you can fix that up, I think we can finally accept this work. 👍

[mingyuan-xia]

Ok I will pull from the master and please squash my commits into one to make things more concise.

[Lukasa]

Great, thanks for this!