Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4)

Issue 14105014: Fix disabled InstallerStateTest.RemoveOldVersionDirs on x64 (Closed)

Created:
7 years, 7 months ago by Will Harris
Modified:
7 years, 7 months ago
Reviewers:
robertshield
CC:
chromium-reviews, grt+watch_chromium.org, jschuh
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Fix InstallerStateTest.RemoveOldVersionDirs by fixing the comparison in alternate_version_generator.cc to use max(DWORD) rather than max(size_t) since CreateFileMapping takes a DWORD. BUG=236376 TEST=installer_util_unittests.exe --gtest_filter=InstallerStateTest.RemoveOldVersionDirs Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=197434

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -11 lines) Patch
M chrome/installer/test/alternate_version_generator.cc View 2 chunks +2 lines, -2 lines 1 comment Download
M chrome/installer/util/installer_state_unittest.cc View 1 chunk +1 line, -9 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Will Harris
small fix, this makes these tests pass and they can be re-enabled on x64.
7 years, 7 months ago (2013-04-29 19:49:28 UTC) #1
robertshield
https://codereview.chromium.org/14105014/diff/1/chrome/installer/test/alternate_version_generator.cc File chrome/installer/test/alternate_version_generator.cc (right): https://codereview.chromium.org/14105014/diff/1/chrome/installer/test/alternate_version_generator.cc#newcode182 chrome/installer/test/alternate_version_generator.cc:182: static_cast<int64>(std::numeric_limits<DWORD>::max())) { Really curious: how does this fix the ...
7 years, 7 months ago (2013-04-29 20:04:45 UTC) #2
Will Harris
On 2013/04/29 19:49:28, Will Harris wrote: > small fix, this makes these tests pass > ...
7 years, 7 months ago (2013-04-29 20:13:15 UTC) #3
robertshield
lgtm
7 years, 7 months ago (2013-04-30 14:16:32 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/wfh@chromium.org/14105014/1
7 years, 7 months ago (2013-04-30 16:25:08 UTC) #5
commit-bot: I haz the power
7 years, 7 months ago (2013-04-30 19:50:22 UTC) #6
Message was sent while issue was closed.
Change committed as 197434

Powered by Google App Engine
This is Rietveld 408576698