chriscz / pysorter

A command line utility for organizing files and directories according to regex patterns.
Mozilla Public License 2.0
44 stars 17 forks source link

implementation of issue_14: dry run flag #25

Closed toondegryse closed 7 years ago

toondegryse commented 8 years ago

If you could assist me to run the test .. :)

chriscz commented 8 years ago

To run the tests locally you can do python setup.py test. To see why the tests failed you can click the Details link next to the travis-ci icon.

Your tests seem to fail due to a bad import. Run tests locally before pushing, as discussed in the Contributing document.

toondegryse commented 8 years ago

Hi Chriscz,

I haven't added an extra import and I can't really locate the bad import problem. For the testing, I suppose I need to compare my output to the expected output, yes? I don't see there being a use for @pytest.mark.xfail?

Thanks, Toon

chriscz commented 8 years ago

Your import used helper instead of pytest. What you should do is capture, using the capsys fixture and then compare output.

There is an example in the source. Are you ruining your tests locally? Because that will help you spot the error.

Let me know if you get stuck! Thanks for helping out

On Tue, 18 Oct 2016, 06:16 Toon Degryse, notifications@github.com wrote:

Hi Chriscz,

I haven't added an extra import and I can't really locate the bad import problem. For the testing, I suppose I need to compare my output to the expected output, yes? I don't see there being a use for @pytest.mark.xfail?

Thanks, Toon

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/chriscz/pysorter/pull/25#issuecomment-254402587, or mute the thread https://github.com/notifications/unsubscribe-auth/AE-lR3vbthAcY91CcABtYt5aVCAdC20rks5q1Eg6gaJpZM4KVbTv .

toondegryse commented 7 years ago

Hi Chriscz,

I am stuck but I really want to do it. I am running the tests locally now. How can I produce output of a test, is that only possible way by using assert? If I use assert (like in the print version test), where does the output go?

Thanks in advance, Toon

chriscz commented 7 years ago

Hey Toon,

As in the print version test, your test function should have a single argument capsys, this will capture any outputs from the function.

To read the output you can exectue out, err = capsys.readouterr() Here are some examples you can look at

When you use assert, it simply checks whether the condition holds, if it doesn't then the test fails. Maybe I'm misunderstanding you.

If you want to see what the output of the test itself is, you can use the -s switch like so:

pytest -s # instead of using python setup.py test
toondegryse commented 7 years ago

Thanks Chriscz,

Thanks for your help. almost there. I have implemented the capsys part but it seems like I need d to create the temporary directories which I can't do if I use capsys.

Toon

chriscz commented 7 years ago

Hey Toon,

Yea, the annotation break's pytest injection. This is my fault. The easiest way to get a temporary directory is to use this syntax:

with TempDirectory() as d:
   # do something with d
toondegryse commented 7 years ago

Hi Chriscz,

This helps me a little but I can't seem to get around this OSError: Directory to organize does not exist or is a file: sourcefiles/

def test_print_changes(capsys):
    with TempDirectory() as d:
        filetypes = {
            ".*\\.pdf$': 'docs/"
        }
        to_sort = "sourcefiles/"
        to_make = ["file1.pdf"]
        helper.initialize_dir(d, filetypes, helper.build_path_tree(to_make, to_sort))

        args = [to_sort, "-n"]
        pysorter.main(args)

        out, err = capsys.readouterr()
        assert out == "move file sourcefiles/file1.pdf --> docs/file1.txt"

Do you have input on this? I have tried a couple of ways around this issue but haven't been successful.

Using following example returns the same error: d.write('sourcefiles/test.txt', b'some foo thing') Thanks, Toon

chriscz commented 7 years ago

Hi Toon,

Please push your WIP to your repository (work in progress) so I can better assist you. I'd suggest you debug a bit more and try and see why it's not working.

It would help a lot if you provide a stack trace / error traceback, because without that there's no way for me to tell what went wrong.

The directory should have been created by initialize_dir. I'm not sure why this is happening

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-1.8%) to 94.737% when pulling e1ad21cde544156ec779ba16db473fee4af8381a on toondegryse:master into a8a3ecb0ecc64a302c776b47479d07b911a157c6 on chriscz:develop.

toondegryse commented 7 years ago

Hi Chriscz,

I have split the capsys from the tmpdir and that seemed to solve my problem. Please have a look.

Toon

chriscz commented 7 years ago

That's interesting. I'll have a look! Good job.

chriscz commented 7 years ago

Hi Toon,

I have looked at your most recent pull request, but that does not have any of your changes on. Please push your changes to this PR.

chriscz commented 7 years ago

Hi toon, why do you keep opening new PR's? This one is the one you should be pushing too.

Anyway I'll look at the other one

toondegryse commented 7 years ago

Hi Chriscz,

Apologies for creating new PR's. Is there a good reason that the edited lines in pysorter.py seem to decrease the coverage?

Toon

chriscz commented 7 years ago

It happens when new code is added, that hasn't been fully tested, you can look at the coverage report to see where some coverage is missing.

The CONTRIBUTING document discusses it. I hope to get around to your submission within the next week.

On Tue, 8 Nov 2016 07:27 Toon Degryse, notifications@github.com wrote:

Hi Chriscz,

Apologies for creating new PR's. Is there a good reason that the edited lines in pysorter.py seem to decrease the coverage?

Toon

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/chriscz/pysorter/pull/25#issuecomment-259049110, or mute the thread https://github.com/notifications/unsubscribe-auth/AE-lR7-tVXzppaDVU5eCMBRBMa4-5y8mks5q8AgogaJpZM4KVbTv .

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.3%) to 96.809% when pulling 950b7f4dba25032be1dde167342ef444130d6a3f on toondegryse:master into a8a3ecb0ecc64a302c776b47479d07b911a157c6 on chriscz:develop.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.3%) to 96.809% when pulling 950b7f4dba25032be1dde167342ef444130d6a3f on toondegryse:master into a8a3ecb0ecc64a302c776b47479d07b911a157c6 on chriscz:develop.

toondegryse commented 7 years ago

Hi Chrisz, have you had a look and/or implemented this on your master branch?

Thanks, Toon

chriscz commented 7 years ago

Hi Toon,

I have not got around to it yet. I'll let you know when I do.

Chris

chriscz commented 7 years ago

Hey Toon.

I'm working on the merge. I'll be pushing it soon.

chriscz commented 7 years ago

Merging completed