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

Implement property 'Personal Note' #142

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CachingFoX
Copy link
Contributor

I expanded the class Cache to read the Personal Note or Cache Note from a cache listing. The note can be read from the listing and from the print form.

There is a new property in the class Cache called 'personal_note' including the setter. I add a new test case and cassette.

Copy link
Owner

@tomasbedrich tomasbedrich left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

Isn't this a premium only feature? I cannot see the personal note field where I expect it according to your test cassette.

gc-note

with self.subTest("Personal Note"):
with self.recorder.use_cassette('cache_personal_note'):
cache = Cache(self.gc, "GC1FPN1") # a location less cache
self.assertEqual("Wandern, Wandern, Wandern, ...", cache.personal_note)
Copy link
Owner

Choose a reason for hiding this comment

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

This is a bit problematic, because the personal note will be different for everyone who is re-running this test (= re-recording a cassette).. I am not sure, how (and whether) we should test this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you are right Personal Note is a PM feature.

If I record the cassette for my test case I have the same question, what happens if some other records the cassette. D'am! I have no real idea how we can solve this problem.

By the way, I have a similar problem for a still existing test case with compares the number of founds. After I updated the cassette the test case fails.

Copy link
Owner

Choose a reason for hiding this comment

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

Let's not test it, since we don't know how.

Feel free to delete offending tests as well. But for example, testing number of founds can be re-written to assert num_of_founds >= 100 (which presumes non-descending tendency of this number and tests correct parsing at least). So please delete only those, which really cannot be fixed.

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