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

Test case for issue #94 - Multiple occurences of year in license #97

Closed
wants to merge 1 commit into from
Closed

Test case for issue #94 - Multiple occurences of year in license #97

wants to merge 1 commit into from

Conversation

mdeweerd
Copy link
Contributor

Test case fro issue #94 .

I triesd a simple fix, but this impacts the other test cases. So it's best that the codeowner checks this.

@Lucas-C
Copy link
Owner

Lucas-C commented Jan 15, 2024

Hi @mdeweerd

Thank you very much for contributing to this project and attempting to fix an existing issue!

I triesd a simple fix, but this impacts the other test cases. So it's best that the codeowner checks this.

I checked the pipeline execution, and the only tests that are failing are the ones related to your new tests cases.

You may have been mislead in thinking that other tests are failing by this output:

usage: pytest [-h] --whitespaces-count WHITESPACES_COUNT
              [filenames [filenames ...]]
pytest: error: argument --whitespaces-count: invalid int value: ''
usage: pytest [-h] --whitespaces-count WHITESPACES_COUNT
              [filenames [filenames ...]]
pytest: error: argument --whitespaces-count: expected one argument
usage: pytest [-h] --whitespaces-count WHITESPACES_COUNT
              [filenames [filenames ...]]
pytest: error: argument --whitespaces-count: invalid int value: 'a.b'
usage: pytest [-h] --whitespaces-count WHITESPACES_COUNT
              [filenames [filenames ...]]
pytest: error: argument --whitespaces-count: invalid int value: 'a/b'

But this is normal output, due to tests in tests/remove_tabs_test.py and the use of the -s Pytest option in .pre-commit-config.yaml.

I have just added a commit to get rid of this -s / --capture=no option so if you rebase this PR, you should not see such misleading debugging output anymore.

@mdeweerd
Copy link
Contributor Author

Thank you for your quick feedback.

Yes, my intention is that these test cases validate the feature.

When I tried to amend the code, I ran into other test cases failing - the way I tried to fix it had too many side-effects and I had other tests that were failing.

I think that with the knowledge about how the tool is constructed, a better approach to fix it than mine is easier to determine.

@Lucas-C
Copy link
Owner

Lucas-C commented Jan 15, 2024

I think that with the knowledge about how the tool is constructed, a better approach to fix it than mine is easier to determine.

As you wish 🙂
Feel free to close this PR if you do not want to continue working on this.

On my side, I don't know when I'll take the time to try to solve this issue,
but I always try to review contributions quickly,
so other PRs to fix this are welcome!

@mdeweerd
Copy link
Contributor Author

Ok, I do not have time to understand this codebase enough to fix this given the experience with the first attempt to fix it.

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