TelegramBots / Telegram.Bot.Extensions.LoginWidget

Allows you to generate embed JavaScript for the Telegram login widget and verify the hashes received
MIT License
32 stars 12 forks source link

Fix LoginWidget #2

Closed karb0f0s closed 6 years ago

karb0f0s commented 6 years ago
karb0f0s commented 6 years ago

Tests require some modification. Hash should be provided to check invalid values. And something wrong with auth_date: CheckAuthorization always returns Authorization.TooOld for test data set. Also it would be nice to slightlymodify tests in order to follow unit-test best practice - AAA (Arrange, Act, Assert) pattern. Unit test basics

karb0f0s commented 6 years ago

Also I have concerns regarding TestDataGenerator. This project could (or even should) be converted to fixture class. This would give such advantages as:

MihaZupan commented 6 years ago

The overloads were not Linq because of love for performance - being able to set the initial capacity parameter in the Dictionary constructor improves the performance of that conversion by 40 ish % (also saves some memory). - Not noticeable, but theoretically there.

CheckAuthorization would return TooOld for test data because the test data is generated a while ago. Because of this, the tests set the AllowedTimeOffset in the constructor. Does running the tests fail for you? Invalid hashes are also checked because InvalidTests are currently generated from valid ones by making changes, some of those affecting the hash as well.

I am not familiar with the test environment enough but having the test data generated on the fly would be better.