Skip to content
This repository was archived by the owner on Dec 12, 2024. It is now read-only.

Slightly better asserts. #452

Merged
merged 3 commits into from
Sep 24, 2020
Merged

Slightly better asserts. #452

merged 3 commits into from
Sep 24, 2020

Conversation

stephen-hawley
Copy link
Contributor

Slightly better. Addressing this issue.
Past me clearly thought that this was going to be used more often than it is, but it's not fully YAGNI (you ain't gonna need it).
Future me may yet be grateful for past me's foresight.

Copy link

@spouliot spouliot left a comment

Choose a reason for hiding this comment

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

minor

@@ -23,7 +23,7 @@ public void LoadsDatabaseSmokeTest (PlatformName platform, int expectedLowerLimi
var database = importer.Import ();
Assert.IsNotNull (database, $"null database for {platform}");
Assert.Less (expectedLowerLimit, database.Count, $"Expected at least {expectedLowerLimit} db entries, but got {database.Count} entries.");
Assert.IsTrue (!errors.AnyErrors, $"{errors.ErrorCount} errors importing database.");
errors.AssertNoErrors ("errors importing database.");

Choose a reason for hiding this comment

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

an error with $"{errors.ErrorCount} errors while {whileDoing}" and "errors importing database." will give you

"1 errors while errors importing database."

@@ -23,7 +23,7 @@ public void LoadsDatabaseSmokeTest (PlatformName platform, int expectedLowerLimi
var database = importer.Import ();
Assert.IsNotNull (database, $"null database for {platform}");
Assert.Less (expectedLowerLimit, database.Count, $"Expected at least {expectedLowerLimit} db entries, but got {database.Count} entries.");
Assert.IsTrue (!errors.AnyErrors, $"{errors.ErrorCount} errors importing database.");
errors.AssertNoErrors ("importing database.");

Choose a reason for hiding this comment

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

Sanity check - would it make sense to make the "importing database" a constant for these tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! PRs welcome! 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants