Skip to content
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

Renamed mock data and associated tests to shorten file names #100

Merged
merged 2 commits into from
Jul 27, 2016

Conversation

MattHyman
Copy link
Member

@MattHyman MattHyman commented Jul 27, 2016

Renamed mock data and associated tests to shorten file names by introducing the concept of a friendly operating system version to refer to mock file names instead of the full OS version. For the current Xbox mock tests this allows replacing "14385_1002_amd64fre_rs1_xbox_rel_1608_160709_1700" with just "rs1_1608" resulting in file names being shortened from _XboxOne_14385_1002_amd64fre_rs1_xbox_rel_1608_160709_1700 to _XboxOne_rs1_1608. The traditional OS Version is still needed when testing the DevicePortal's OperatingSystemVersion property.

@msftclas
Copy link

Hi @MattHyman, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Matt Hyman). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

/// <summary>
/// Text to identify an xbox rel build.
/// </summary>
private static readonly string XboxRelText = "xbox_rel_";
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't love this. Feels very xbox specific when this should work for all devices.

Maybe instead of looking for fre, you could work backwards? Basically remove everything from the last period on, then remove everything from the last period forward. So you go from this:
14385.1002.amd64fre.rs1_xbox_rel_1608.160709-1700
to this:
14385.1002.amd64fre.rs1_xbox_rel_1608
to this:
rs1_xbox_rel_1608.

Then we can either keep with that, or if you really want to drop to just rs1_1608 maybe we can drop anything between underscores?

So if another platform OS came out of rs1_release, they'll just get rs1_release since there are no extra underscores, but if they shipped out of, say, rs1_onecore_dep_dart_dev1, they'd get the nice short rs1_dev1 ?

I wish I knew a bit more about other devices shipping branches to be 100% confident this would always function well, but I think that's a decent start without anything xbox specific.

Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that Xbox specific is not the right option here. Let us just go down to the branch name, in this case rs1_xbox_rel_1608, to avoid overcomplicating things and backing us into a corner in the future. That being said, the concept of an "friendly OS version" gives us flexibility to change this scheme in the future but leave existing tests/files alone.

…f rs1_1608 to ensure that the logic works for non-Xbox branches. Also renamed related files.
@WilliamsJason WilliamsJason merged commit efbc867 into master Jul 27, 2016
@MattHyman MattHyman deleted the HymanMatt branch July 28, 2016 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants