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

Improve tests #199

Open
ghost opened this issue Oct 9, 2021 · 10 comments
Open

Improve tests #199

ghost opened this issue Oct 9, 2021 · 10 comments
Labels
good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed

Comments

@ghost
Copy link

ghost commented Oct 9, 2021

We have a lot of tests that were just copy-pasted from each other. Because of that, test files are long.

What do you need to do?
Take a look at our tests and try to add some helper functions, make tests shorter.

@ghost ghost added good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed labels Oct 9, 2021
@deepakdinesh1123
Copy link

Hi, can I take this?

@ghost
Copy link
Author

ghost commented Oct 10, 2021

Hi, can I take this?

Apparently, there is already draft PR: #202

@jyooru
Copy link
Contributor

jyooru commented Oct 10, 2021

Right now I'm only working on test_regex_identifier.py, so if you want you can work on some other files @deepakdinesh1123!

@PabloLec
Copy link
Contributor

And while we are discussing tests files, I'm not sure about the usefulness of some tests in test_click.py. We do test that all regex work in test_regex_identifier.py and #202 plans to make it easier which is great. But there is a lot of repetition in test_click.py, every regex is also tested there. Either as an arg or from the fixture file.
I added some tests myself, as I seemed to be the way it was done but I'm not sure it is useful and pretty sure it could at least be simplified.
Any thoughts? Maybe I'm just missing something.

Originally posted by @PabloLec in #206 (comment)

When #202 gets merged with examples alongside in the database, we could maybe take those, write it to a file and run the file against pyWhat to verify that regex works if match is put in a file? But we also need to test boundaryless, so matches in between of something else..

Originally posted by @amadejpapez in #206 (comment)

@PabloLec
Copy link
Contributor

I guess testing boundaryless and every other CLI arguments could still be made in test_click.py of course. Probably against a permanent fixture file, meaning we would not have to add every example to this fixture file.
Since everything tricky will be tested, we know the parsing part works and we could only test regex one by one with #202 method.

And we could still make a test to just iterate over all regex and respective examples and pass them as args to make sure everything works fine.

@Saacket
Copy link

Saacket commented Oct 17, 2021

hello

@jyooru
Copy link
Contributor

jyooru commented Oct 18, 2021

I'm starting to work on parameterizing test_click.py now.

And we could still make a test to just iterate over all regex and respective examples and pass them as args to make sure everything works fine.

I could implement this.

@ghost
Copy link
Author

ghost commented Nov 3, 2021

Hey @jyooru, are you still working on click tests?

@jyooru
Copy link
Contributor

jyooru commented Nov 4, 2021

Sorry, I forgot about this. I've cleaned up a little bit on my branch refactor/test/click.

After looking at my changes, I think that passing each regex as args is going to be a better idea than continuing to use a fixture file. This also means that each test could be automatically generated like in #202, instead of using a manually updated list that I implemented on my branch.

I'm happy to implement each regex being passed as args.

@ghost
Copy link
Author

ghost commented Nov 4, 2021

Sorry, I forgot about this. I've cleaned up a little bit on my branch refactor/test/click.

After looking at my changes, I think that passing each regex as args is going to be a better idea than continuing to use a fixture file. This also means that each test could be automatically generated like in #202, instead of using a manually updated list that I implemented on my branch.

I'm happy to implement each regex being passed as args.

Actually, I am thinking that regex tests should be removed from test_click.py since we already test regex database. Only CLI related tests should be left.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers hacktoberfest help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants