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

Fix Imgur Unit Tests #101

Closed
Tjzabel opened this issue Dec 2, 2018 · 4 comments

Comments

@Tjzabel
Copy link
Member

commented Dec 2, 2018

Problem

Imgur unit tests fail

Why?

The TgImgurPhotoHandler class is expecting an imgur object in its constructor. Currently, the mockImgur object is being passed into this parameter in the tests. This causes an error when running yarn test stating:

TgPhotoHandlerTests
✔ TgPhotoHandler_DisabledTest
✔ TgPhotoHandler_EnabledNoCaptionTest
✔ TgPhotoHandler_EnabledCaptionTest
Error when uploading to imgur: undefined
✔ ImgurPhotoHandler_SuccessTest

Possible Solution

I believe this is happening because the mockImgur object does not require('imgur') and so therefore does not pull from its properties. We need to figure out the best way to create a test imgur object.

Secondary Issue

Looking at this line of code in TgPhotoHandlerTests.js:

let photoSmall = {
    file_id : 1,
    file_size : 1,
    url : "https://ritlug.com/test/1.png",
    imgurUrl : "https://imgur.com/small.png"
};

The url field has a broken link. When the RITlug website was redesigned, these test images did not transfer over to the new site.
We have added new assets to be used in place of these broken test images.

Secondary Solution

This is an easy fix
Simply replace the https://ritlug.com/test/#.png links with the raw.githubusercontent.com links from each of the asset images.

@Tjzabel Tjzabel added this to the v1.3 milestone Dec 2, 2018

@Tjzabel Tjzabel changed the title Fix Imgur Tests Fix Imgur Unit Tests Dec 2, 2018

@Tjzabel

This comment has been minimized.

Copy link
Member Author

commented Dec 2, 2018

Update

After some debugging, it is still unclear whether or not this issue is related to mockImgur or not.

The guilty test case is the ImgurPhotoHandler_SuccessTest, specfically the second test case working with a photo that does not contain an imgur link.

When running only the first imgur test method, the undefined imgur error does not appear. This should narrow down the error to just the second test case within these imgur tests.

Debugging is still an ongoing process.

@xforever1313

This comment has been minimized.

Copy link
Contributor

commented Dec 2, 2018

Unit tests are supposed to mock out imgur, so require('imgur') is not needed. If the test is failing it could be because MockImgur doesn't have a function that we need to mock that we need to add.

The URL at ritlug.com never existed. Because we mock out imgur, it never actually queried ritlug, and never actually uploaded anything to imgur. The purpose of those unit tests is if imgur were to return a successful JSON message, does the correct message get relayed to IRC. The other unit test is if imgur were to return a failure by throwing an exception, does the ImgurHandler handle it okay.

photoNotInImgur purposefully does not have an imgurUrl field so MockImgur's upload URL does the failure case.

If we want to actually want to query ritlug.com and imgur, it is no longer a unit test; it is an integration/acceptance test.

@Tjzabel

This comment has been minimized.

Copy link
Member Author

commented Dec 2, 2018

@xforever1313 ah, thanks for the overview!
After the update, I had realized Imgur was not required.

There must be some other method that has not yet been overridden.

@Tjzabel

This comment has been minimized.

Copy link
Member Author

commented Dec 9, 2018

Update

So, after extensive debugging, I've come to the conclusion that there's nothing really to fix. The only issue I see, is that when calling getFileLink from the mockTgBot, it returns undefined when calling the photoNotInImgur object because this object does not exist as one of its files. I'm not sure this is the desired functionality. I think it's supposed to return an undefined error when "uploading" to imgur, not because it can't find the tgUrl.

@xforever1313 is this supposed to be the case? Or is it supposed to return an error because it doesn't have an imgur url?

Either way, both options have the same end result, with Error when uploading to imgur: undefined being printed to the console.

Tjzabel added a commit to Tjzabel/teleirc that referenced this issue Dec 12, 2018
Tjzabel added a commit to Tjzabel/teleirc that referenced this issue Dec 12, 2018
Tjzabel added a commit to Tjzabel/teleirc that referenced this issue Dec 12, 2018
Fix imgur tests, and add new ones (closes RITlug#101).
Remove unneeded test images (sorry @jwflory).
Tjzabel added a commit to Tjzabel/teleirc that referenced this issue Dec 12, 2018
Fix imgur tests, and add new ones (closes RITlug#101).
Remove unneeded test images (sorry @jwflory).

@Tjzabel Tjzabel closed this in fecf6e4 Dec 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.