liuggio / fastest

Simple parallel testing execution... with some goodies for functional tests.
MIT License
475 stars 65 forks source link

before option not working anymore #151

Closed matlar83 closed 4 years ago

matlar83 commented 4 years ago

With the last version (v1.7.0) the --before command option seems not to work anymore.

Steps to reproduce: with version 1.6.1:

> rm -f before.txt
> ls | fastest --before=\"echo hello >> before.txt\" \"echo {}\"
> ls -l before.txt
-rw-r--r-- 1 matlar matlar 10 giu 12 11:53 before.txt

Status OK: before command is executed

with version 1.7.0:

> rm -f before.txt
> ls | fastest --before=\"echo hello >> before.txt\" \"echo {}\"
> ls -l before.txt
ls: cannot access 'before.txt': No such file or directory

Status KO: before command is not executed

Some other issues related to --before option, also in v.1.6.1:

DonCallisto commented 4 years ago

@matlar83 can you try and see if https://github.com/liuggio/fastest/tree/input_args_parsing fix this? I've only fixed args parsing, the "older" issues with --before should not be trackled here. Let me know please. Thank you.

matlar83 commented 4 years ago

@DonCallisto, it does not fix it, according to my tests. And it seems not to run the "execute" command anymore.

Performed test:

> rm -f before.txt test.txt
> ls | fastest --before="echo ciao >> before.txt" "echo test {} >> test.txt"
> ls -l before.txt test.txt
ls: cannot access 'before.txt': No such file or directory
ls: cannot access 'test.txt': No such file or directory

Can you reproduce it? Thank you

Brightside56 commented 4 years ago

I confirm, -b/--before isn't working

DonCallisto commented 4 years ago

I was able to reproduce. With my fix, I'm not using the StringInput the right way: as a matter of fact, it expects to match a certain input, so a first try is to copy/paste the function that parse input string directly in fastest but with no luck. I'm not sure why the command is not interpreted in the right way. Sadly last week, I had no time to investigate deeper. I'll try this week but I really don't know if I'll have time.

If someone would like to help (with suggestions I gave before) is welcome to do so.

DonCallisto commented 4 years ago

@matlar83 @Brightside56 sorry for the long delay, I was quite busy. I've updated my fork https://github.com/liuggio/fastest/tree/input_args_parsing

Could you give it a shot and let me know if it's ok for your projects? Mine tests were fine.

DonCallisto commented 4 years ago

If no one will give a feedback, I'm gonna merge the PR within this week and release a new tagged version in order to let everyone use new code. I'm pretty confident it's gonna be fine but a double check would be great. Thanks in advance @matlar83, @Brightside56 and everyone will check this.

matlar83 commented 4 years ago

Sorry @DonCallisto, I don't have time to test it :-(

I hope I will have in the next weeks, but I'm not sure

DonCallisto commented 4 years ago

Don't worry. I've used your test cases as guidelines and it works like a charm, so I'm pretty sure we won't have surprise here. After all, this taken me 4 months, it won't be a problem if you can't try it.

DonCallisto commented 4 years ago

I've released v1.7.1 https://github.com/liuggio/fastest/releases/tag/v1.7.1 It should fix the bug.

DonCallisto commented 4 years ago

Gonna close it for the moment. Feel free to comment if something's wrong and I'll reopen it.