Skip to content

Conversation

@sminnee
Copy link

@sminnee sminnee commented Aug 31, 2020

Creating this patch was quite simple: empty the tests/fixtures/ path and then re-execute the test suite to recreate them. It should get the suite to pass.

It does highlight that the suite is kind of brittle and begs the question - how much is being gained by testing all of these different 3rd party systems? What behaviour of the module is being tested here?

However, that's a much bigger question so I did this to begin ;)

@oscarotero
Copy link
Collaborator

Yep, the problem is now the test does not work in my local. This is something that I don't understand, because, in theory no real requests should be made, all data should be fetched from cached responses.

I know that this testing method is not perfect. I'd want to test real use cases, because there many different cases:

  • There are adapters to specific sites (archive.org, facebook, github, pinterest, etc). These sites are changing the html code continually, so it's a way to make a snapshot of the code that can be easily updatable.
  • There are other sites without adapters, but containing some peculiarities that are important to test.

Anyway, I'm open to other testing suite.

@sminnee
Copy link
Author

sminnee commented Aug 31, 2020

Aaah, that makes more sense. I think that root cause is the cache usage is broken in both my local and travis. I'll look into that

@sminnee
Copy link
Author

sminnee commented Aug 31, 2020

OK I've confirmed that the cache was working for me locally.

I've dug into a couple of small examples to try and shed some light:

Looking at this commit, it adds a 'ratio' property to all results.
08b66b6

However the ratio property isn't added to the GitHub fixtures.

Here in tests/fixtures/archive.org.details-dn2015-0220_vid.php the URL changes
08b66b6#diff-595d297878db8096a1bdc73d794e58ba

But the reference in /tests/cache/archive.org.details-dn2015-0220_vid.068080477804db642734702dac463001.php hasn't changed.

Do you know why that might be? Is it possible that some files were missed from that commit?

@oscarotero
Copy link
Collaborator

Yes, I've added the ratio property and update the tests to include this new value to the fixtures. But the tests were failing even before this change.

The new tests fixtures (updated in this PR) are failing in my local machine. See this screenshot:

imaxe

There're only two differences, but I cannot see why this happens. It seems magic 😄

@sminnee sminnee mentioned this pull request Oct 12, 2020
@oscarotero oscarotero closed this Oct 27, 2020
@sminnee sminnee deleted the pulls/fix-test-fixtures branch October 28, 2020 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants