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

Clitest update #1872

Merged
merged 1 commit into from Jun 3, 2016
Merged

Clitest update #1872

merged 1 commit into from Jun 3, 2016

Conversation

@jphilipsen05
Copy link
Contributor

@jphilipsen05 jphilipsen05 commented Jun 2, 2016

There was a bad if/elif statement and the elif os.path ... would never get hit. So I swapped the order of those two statements and added testing for the prepare_exec_for_file function.

@ThiefMaster
Copy link
Member

@ThiefMaster ThiefMaster commented Jun 2, 2016

second commit should be amended into the first one

also, the first line of a commit should be no longer than 51 chars (right not even github truncates it) - see http://chris.beams.io/posts/git-commit/ for some useful guidelines

Loading

@jphilipsen05 jphilipsen05 force-pushed the clitest_update branch 2 times, most recently from 2e0d78a to 0355d93 Jun 2, 2016
Also update relevant test
@untitaker
Copy link
Member

@untitaker untitaker commented Jun 3, 2016

@ThiefMaster We can squash ourselves so commit styling is no longer that relevant.

@jphilipsen05 Can you verify (on your machine) that the tests fail without the bugfix?

Loading

@untitaker untitaker self-assigned this Jun 3, 2016
@jphilipsen05
Copy link
Contributor Author

@jphilipsen05 jphilipsen05 commented Jun 3, 2016

Yes, If I swap the if/elif statements back to the original I get this error in the tests. Showing that the first if statement for an init.py is hitting the if statement that handles python files and does not reach the expected init.py elif statement.

When I have my bug fix in place there are no errors.

Let me know if you have any other questions.

test_apps = None

    def test_prepare_exec_for_file(test_apps):
        assert prepare_exec_for_file('test.py') == 'test'
>       assert prepare_exec_for_file('/usr/share/__init__.py') == 'share'
E       assert '__init__' == 'share'
E         - __init__
E         + share

Loading

@untitaker untitaker merged commit fe5f714 into pallets:master Jun 3, 2016
1 check passed
Loading
untitaker added a commit that referenced this issue Jun 3, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants