interfasys / mediametadata

A cloud application which provides CRUD access to the metadata stored in images
GNU Affero General Public License v3.0
5 stars 1 forks source link

Issue #16: Added code for extraction of EXIF Data #33

Closed imjalpreet closed 8 years ago

oparoz commented 8 years ago

Ok, so there is a warning because you're adding an assertion on a private method called extractImageDimensions

imjalpreet commented 8 years ago

@oparoz Do you have any solution in your mind? Also, I have also written the code for the remaining fields on my system just waiting for this to get merged.

Secondly, I have also tried and found out ways to write the EXIF and IPTC data. I have written a sample code and it worked. So, just this test is the main obstacle right now.

oparoz commented 8 years ago

I set the method to public like you said you did and it seems to work. Codeception now complains about the fact that extractImageDimensions is never called, which is another problem.

oparoz commented 8 years ago

You're missing a test anyway in that method. It returns the metadata and you need to make sure that what is returned is what you expect it to be.

imjalpreet commented 8 years ago

@oparoz I have been getting the same error even when it is private and when I changed it to public. I never got any warning about being a private method.

1) ExtractMetadataTest: Extract
 Test  tests/unit/Services/ExtractMetadataTest.php:testExtract
Expectation failed for method name is equal to <string:extractImageDimensions> when invoked 1 time(s).
Method was expected to be called 1 times, actually called 0 times.
imjalpreet commented 8 years ago

@oparoz I had checked the exif data and as the above part was failing so I wanted it to pass, that's why I did not put a check for metadata yet but I will add it once this passes.

oparoz commented 8 years ago

Which method did you set to public?

imjalpreet commented 8 years ago

@oparoz I set extractImageDimensions to public but I did not push the code as it gives the same failure message that it gives when it is private.

oparoz commented 8 years ago

Please push that change so that I can test the same code

oparoz commented 8 years ago

I've just realised what the problem is. You're mocking the method you're testing. That is never going to work.

You need to mock getimagesize like we discussed in your 1st test PR

imjalpreet commented 8 years ago

@oparoz I have mocked getimagesize just like that as we discussed in 1st test PR. Can you check if I did it wrong?

oparoz commented 8 years ago

I also just tried with your latest push and get the same error, so I don't know why you're not getting it on your setup.

1) ExtractMetadataTest: Extract
 Test  tests/unit/Services/ExtractMetadataTest.php:testExtract
Expectation failed for method name is equal to <string:extractImageDimensions> when invoked 1 time(s).
Method was expected to be called 1 times, actually called 0 times.
imjalpreet commented 8 years ago

@oparoz I am also getting the same error from the starting.

oparoz commented 8 years ago

I have mocked getimagesize just like that as we discussed in 1st test PR. Can you check if I did it wrong?

Ah, yes, I see it now at the top

So it should just be a matter of checking the returned value to see if it matches what you expect

oparoz commented 8 years ago

Of course, you should remove

        $this->extractor->expects($this->once())
            ->method('extractImageDimensions')
            ->with($absolutePath)
            ->willReturn($dimensions);
imjalpreet commented 8 years ago

@oparoz Is there a problem if I don't check the returned value? I mean does the test not pass till I check the returned value as it is failing due to not calling extractImageDimensions method.

imjalpreet commented 8 years ago

@oparoz Okay but why should that not be tested? I mean you told me to test every method call that's why I had written that expect.

oparoz commented 8 years ago

OK, come to IRC and I'll answer your questions :)