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

AppleDict-bin: add new tests #436

Closed
wants to merge 2 commits into from

Conversation

soshial
Copy link
Contributor

@soshial soshial commented Feb 24, 2023

@ilius
Copy link
Owner

ilius commented Feb 24, 2023

What are all these Addr[...] in txt files?

@ilius ilius force-pushed the appledict-bin-keytext-tests branch from 5ca4a72 to 048c767 Compare February 24, 2023 16:37
@soshial
Copy link
Contributor Author

soshial commented Feb 24, 2023

There was some confusion, @ilius. These *.TXT files that I created are specifically to test only extracted KeyText.data! They were NOT supposed to be the tab files with the result of full conversion as in all other tests.

This PR contains test logic (which I wasn't able to run because of file downloading) and with several fixes (!). Please have a look at this code: https://github.com/soshial/pyglossary/blob/appledict-bin-keytext-tests/tests/g_appledict_bin_test.py#L278-L284

@ilius
Copy link
Owner

ilius commented Feb 24, 2023

Please don't write these kind of tests.
We shouldn't even access internal methods of plugin in tests.
Write functional tests like we have for other plugins.

@ilius
Copy link
Owner

ilius commented Feb 24, 2023

These kinds of tests are prune to break when you refactor the plugin.

@ilius
Copy link
Owner

ilius commented Feb 24, 2023

I implemented and test progress bar for KeyText.data in this branch:
https://github.com/ilius/pyglossary/tree/apple-bin

Can you bring the changes here?

It's fine that it's a separate progress bar for open.
It's actually better and more clear.

@ilius
Copy link
Owner

ilius commented Feb 24, 2023

I changed txt files to Tabfile at
ilius/pyglossary-test#3

@soshial
Copy link
Contributor Author

soshial commented Feb 24, 2023

Unfortunately, your tests are also prone to break, whenever you:

  1. change the XML presentation of entry
  2. make some fixes with string replacements .replace()

I wonder how many times you changed Tabfiles...

Frankly, that is quite sad to hear that you won't accept my tests, that were written to check extraction of data from binary files "as-is". My tests are unit-tests and they would help troubleshoot any occurring problem; while functional tests would only signify that there is some problem out there (really hard to troubleshoot and find a bug quickly).

Would you please merge my fixes of KeyText.data parsing here: pyglossary/plugins/appledict_bin/__init__.py from this PR? They are ipmportant.

@ilius
Copy link
Owner

ilius commented Feb 24, 2023

Yes they break when plugin behaves differently, not when it only changes internally.

https://en.wikipedia.org/wiki/Functional_testing

@ilius
Copy link
Owner

ilius commented Feb 24, 2023

These tests actually test most of the plugin, so they are too big for unit tests.

I'm okay with unit tests, but should be much smaller and require no data file.

@ilius
Copy link
Owner

ilius commented Feb 24, 2023

See this for example:
https://github.com/ilius/pyglossary/blob/master/tests/dsl_test.py

These are unit tests.

But test files that their name starts with g_ are functional tests. By g_ I meant glossary.
They should not import the plugin.

ilius added a commit that referenced this pull request Feb 24, 2023
@ilius ilius force-pushed the appledict-bin-keytext-tests branch from 048c767 to 408b445 Compare February 24, 2023 17:57
@ilius
Copy link
Owner

ilius commented Feb 24, 2023

I pushed your plugin changes to master.

@ilius ilius changed the title AppleDict-bin: fixes and tests AppleDict-bin: add new tests Feb 24, 2023
@soshial
Copy link
Contributor Author

soshial commented Feb 24, 2023

I pushed your plugin changes to master.

Thank you so much for your swift response! Sorry for my curiosity, but which timezone do you live in?

@ilius
Copy link
Owner

ilius commented Feb 24, 2023

UTC+3:30

@soshial soshial closed this Feb 24, 2023
@soshial soshial deleted the appledict-bin-keytext-tests branch February 26, 2023 11:47
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.

None yet

2 participants