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

Improved Game Data API integration tests #198

Merged
merged 39 commits into from
Jul 20, 2021

Conversation

danjagnow
Copy link
Collaborator

Improved all integration tests for the Game Data API by using the test utilities for deep comparison. Also updated model types that were discovered to have issues as a result of these improvements. These are breaking changes, so I'm also updating the version.json to bump the major version.

  • Added a new AssetWithoutFileDataId class and updated the Assets property of CreatureDisplayMedia to use it since the non-nullable FileDataId does not appear to be returned with for this type.
  • Fixed guild crest model types. Added a Colors property to GuildCrestComponentsIndex with a new GuildCrestColors model type to support it. Updated GuildCrestBorderMedia and GuildCrestEmblemMedia to use AssetWithoutFileDataId instead of Asset for their Assets properties.
  • Added a PurchaseQuantity property to the Item model class.
  • Updated JournalMedia to use AssetWithoutFileDataId instead of Asset for the Assets property.
  • Fixed the Mount model type. Added a Requirements property with a new MountRequirements model type to support it.
  • Fixed mythic raid leaderboard model types. Replaced the Zone property of the MythicRaidLeaderBoard model class with a JournalInstance property.
  • Fixed pet model types. Added a missing Media property to Pet with a new PetMediaReference model type to support it.
  • Fixed quest model types. Added a missing MaxCharacterLevel property to QuestRequirements. Removed the RecommendedMinimumLevel and RecommendedMaximumLevel properties from Quest.

Fixed RequestResultAssertions serialization options to use the same converters used by WarcraftClient for consistency. This is important to ensure that TimeSpan and DateTimeOffset properties round-trip properly.

Improved AchievementApiTests by using the test utilities for deep comparison.
Improved AuctionHouseApiTests by using the test utilities for deep comparison.
Improved AzeriteEssenceApiTests by using the test utilities for deep comparison.
Improved ConnectedRealmApiTests by using the test utilities for deep comparison.
Improved CreatureApiTests by using the test utilities for deep comparison.
Added a new AssetWithoutFileDataId class and updated the Assets property of CreatureDisplayMedia to use it since the non-nullable FileDataId does not appear to be returned with for this type.  This is a small change, but a breaking change, so I'm also updating the version.json to bump the major version.
Improved GuildCrestApiTests by using the test utilities for deep comparison.
Fixed guild crest model types.  Added a Colors property to GuildCrestComponentsIndex with a new GuildCrestColors model type to support it.  Updated GuildCrestBorderMedia and GuildCrestEmblemMedia to use AssetWithoutFileDataId instead of Asset for their Assets properties.
Improved ItemApiTests by using the test utilities for deep comparison.
Improved JournalApiTests by using the test utilities for deep comparison.
Updated JournalMedia to use AssetWithoutFileDataId instead of Asset for the Assets property.
Improved ModifiedCraftingApiTests by using the test utilities for deep comparison.
Improved MountApiTests by using the test utilities for deep comparison.
Fixed the Mount model type.  Added a Requirements property with a new MountRequirements model type to support it.
Improved MythicKeystoneAffixApiTests by using the test utilities for deep comparison.
Improved MythicKeystoneDungeonApiTests by using the test utilities for deep comparison.
Fixed RequestResultAssertions serialization options to use the same converters used by WarcraftClient for consistency.  This is important to ensure that TimeSpan and DateTimeOffset properties round-trip properly.
Improved MythicKeystoneLeaderboardApiTests by using the test utilities for deep comparison.
Improved MythicRaidLeaderboardApiTests by using the test utilities for deep comparison.
Fixed mythic raid leaderboard model types.  Replaced the Zone property of the MythicRaidLeaderBoard model class with a JournalInstance property.  Updated the resource file with more current sample data.
Improved PetApiTests by using the test utilities for deep comparison.
Fixed pet model types.  Added a missing Media property to Pet with a new PetMediaReference model type to support it.
Improved PlayableClassApiTests by using the test utilities for deep comparison.
Improved PlayableRaceApiTests by using the test utilities for deep comparison.
Improved PlayableSpecializationApiTests by using the test utilities for deep comparison.
Improved PowerTypeApiTests by using the test utilities for deep comparison.
Improved ProfessionApiTests by using the test utilities for deep comparison.
Improved PvpSeasonApiTests by using the test utilities for deep comparison.
Improved PvpTierApiTests by using the test utilities for deep comparison.
Improved QuestApiTests by using the test utilities for deep comparison.
Fixed quest model types.  Added a missing MaxCharacterLevel property to QuestRequirements.  Removed the RecommendedMinimumLevel and RecommendedMaximumLevel properties from Quest.  Updated the resource file with a more current quest response sample.
Improved RealmApiTests by using the test utilities for deep comparison.
Improved RegionApiTests by using the test utilities for deep comparison.
Improved ReputationFactionApiTests by using the test utilities for deep comparison.
Improved SpellApiTests by using the test utilities for deep comparison.
Improved TalentApiTests by using the test utilities for deep comparison.
Improved TitleApiTests by using the test utilities for deep comparison.
Improved WowTokenApiTests by using the test utilities for deep comparison.
Copy link
Collaborator

@willwolfram18 willwolfram18 left a comment

Choose a reason for hiding this comment

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

Glad to see the deep object comparisons are helping to improve the test coverage and making sure the models align with Blizzard's APIs!

/// <summary>
/// A media asset.
/// </summary>
public record AssetWithoutFileDataId
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm thinking maybe we should make the FileDataId property on the original Asset class a nullable, i.e. long? FileDataId { get; init; }. Then it reduces the maintenance effort if an asset has more fields added that are shared between both models. What are your thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm thinking maybe we should make the FileDataId property on the original Asset class a nullable, i.e. long? FileDataId { get; init; }. Then it reduces the maintenance effort if an asset has more fields added that are shared between both models. What are your thoughts?

Yes, I'm of divided mind about that. It would reduce the number of types and avoid awkward type names with Without in the name. On the other hand, we would sometimes be returning objects with properties that can (for their usage in that particular method) never have a non-null value. I'm not really happy with either scenario. If you'd like to tip the scales one way or another, go for it.

@danjagnow
Copy link
Collaborator Author

I'll merge this as-is for now, but I'm open to reducing the number of nearly-identical types at the cost of some phantom properties that can only be populated in some contexts.

@danjagnow danjagnow merged commit 67ae28b into master Jul 20, 2021
@danjagnow danjagnow deleted the feature/game-data-integration-tests branch July 20, 2021 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants