Add unit tests for isValidDeviceId

[enricogior]

Test for regression #1821

Valid device ids:

DELF016#5&2cf76c78&0&UID8449_1920_1200_{00000000-0000-0000-0000-000000000000}
DELA0A6#5&2cf76c78&0&UID8721_3440_1440_{3A71495C-7321-4BB5-9C79-547C20407CF9}
Default_Monitor#1&1f0c3c2f&0&UID256_1920_1200_{00000000-0000-0000-0000-000000000000}
LOCALDISPLAY_3440_1440_{00000000-0000-0000-0000-000000000000}

Need to verify why we generate LOCALDISPLAY_3440_1440_{00000000-0000-0000-0000-000000000000} even if the code seems to actually add # after LOCALDISPLAY, maybe in this case we already receive LOCALDISPLAY by the Windows API.

PowerToys/src/modules/fancyzones/lib/FancyZones.cpp

Lines 788 to 790 in a6b7ba4

	deviceId = GetSystemMetrics(SM_REMOTESESSION) ?
	L"\\\\?\\DISPLAY#REMOTEDISPLAY#" :
	L"\\\\?\\DISPLAY#LOCALDISPLAY#";

[enricogior]

As suggested by #1826 (comment)

TEST_METHOD (DeviceIdWithoutHashInName)
{
    const auto deviceId = L"LOCALDISPLAY_5120_1440_{00000000-0000-0000-0000-000000000000}";
    Assert::IsTrue(isValidDeviceId(deviceId));
}

[riverar]

I advise against trying to parse device IDs; these may look parsable but they're opaque strings that aren't meant to be parsed.

I'm not familiar with the codebase yet; is there a reason you need to parse these?

[enricogior]

@riverar
the FancyZones device id is the concatenation of three strings:

• the actual physical device id returned by the system (unfortunately is not 100% guarantee it's unique)
• the native screen resolution
• the virtual desktop id

It wasn't the best idea to use underscore as a separator.

Currently the validation is mainly to avoid importing invalid data from the json file (since it's human readable, it's also human editable so we don't assume it's always valid).

We may consider avoiding the validation and keep garbage in the json file, but we may still have to parse it for other purposes.
In the FancyZones editor v2 we are considering to apply custom layouts only to screens with the same resolution of the screen on which they were designed. For the normal case of physical devices that may not require to parse the FZ device id, but we also want some flexibility for non physical devices like virtual machines or RDP and not force the resolution rule. To support those cases we may have to parse the FZ device id.
I'm leaning forward the idea of a breaking change to redesign from scratch how we build and parse the FZ device id, so that we make sure we don't sub-parse the physical device id.
Does it make sense?

[enricogior]

This PR #8235 will make this test obsolete, new tests will be required for the updated monitor ID structure.

[crutkas]

@SeraphimaZykova is this still relevant?

[SeraphimaZykova]

@SeraphimaZykova is this still relevant?

No, this has been done a long ago.