- User Since
- Aug 18 2015, 5:38 AM (283 w, 3 d)
Sep 15 2020
Hi Petr Hosek,
May 20 2020
Nov 28 2017
The previous patch doesn’t support the sequential commands, for example “mkdir existing_folder || true”. This caused sanitizer-windows test failed. The patch has been updated to support the sequential commands.
Nov 23 2017
Reverted rL318911 since it broke the sanitizer-windows.
Nov 20 2017
The test of MC/AsmParser/preserve-comments-crlf.s failed since it uses the unsupported diff option --strip-trailing-cr. I updated the internal diff shell command to support this option.
Nov 10 2017
Optimised the patch by using python ‘getopt’ module.
Nov 9 2017
Updated the builtin diff shell to support ‘-u’, ‘-b’ and ‘-w’ options. Now, all llvm, and clang tests passed with both Python 2 and Python 3.
Nov 7 2017
- I don't see any tests for the case where the commands *succeed*. Only for the case where they fail. Shouldn't we have some tests that verify the commands actually work?
The patch updated the test (valid-shell.txt) where lit internal shell commands succeeded.
Nov 6 2017
Update: I actually do see where you're testing that they work, but I'm not convinced that the tests are sufficient. For several reasons.
- it doesn't test that the builtin command is actually run versus the OS command. It only tests that whatever command is run doesn't fail.
- It doesn't test that the result of the command is as expected, it only tests that it doesn't return an error. For a somewhat unrealistic illustration of this, imagine that all the commands just returned success without actually doing anything. The test as it is written now would pass.
- Now that we have upwards of 6 builtin commands, I'm starting to think they should be separated into their own file. For example, lit/test/ShellBuiltins.py We could test them via unittests (e.g. see how BooleanExpression.py implements its unit tests).
What do you think?
Nov 2 2017
Oct 18 2016
Sep 30 2016
Sep 27 2016
Sep 26 2016
Sep 20 2016
Ok, I have updated the patch following your advice.
Sep 19 2016
I agree with you and abandon this patch.
Sep 16 2016
Sep 14 2016
Sep 13 2016
Sep 12 2016
Sep 9 2016
Sep 8 2016
Sep 5 2016
Sep 2 2016
Thanks for your pseudocode . Two issues have been fixed.
Sep 1 2016
Thanks for your explanation. I have updated the patch following your advice.
Aug 30 2016
Reused the existing native_separator.c test to check fully covered source file case.
Aug 29 2016
Thanks, the test has been added.
Aug 28 2016
Thanks Vedant, the patch has been updated following your comments.
Aug 26 2016
Aug 24 2016
Updating the patch to support the "Goto first zero count" feature.
Aug 23 2016
I have fixed the issues, Thanks.
Aug 16 2016
Aug 11 2016
Many thanks for your comments. As I will be busy on other work in next several days, I will update the patch following your comments once I get those on track. Thanks for the patience.
Aug 10 2016
I have put the change of adding the project summary into Phabricator (https://reviews.llvm.org/D23345).
Aug 9 2016
Fix a linux test.
Aug 8 2016
Aug 3 2016
Following Vedant's comments, all issues have been fixed.
Thanks Vedant, I have updated the patch following your comments.
Aug 2 2016
Jul 26 2016
Jul 24 2016
Thanks for adding the test.
Jul 21 2016
The test in test/tools/llvm-cov/prevent_false_instantiations.h has been updated.
Thanks Vedant. I have updated the patch following your comments.
Jul 20 2016
Jul 6 2016
Jul 4 2016
There are two issues with the current patch on Windows, I am happy to patch these later if you want.
Mar 21 2016
Thanks Harlan, LGTM.
Mar 2 2016
Thanks Sean, the patch looks good to me.
Dec 17 2015
Thanks Paul for your help.
Dec 16 2015
Could someone commit it for me please (as I do not have commit access)? Thanks
Dec 15 2015
Thanks, I have updated the test following your comments.
My patch changes 6 compiler flags, which are –coverage, -fprofile-arcs, -fprofile-generate, -fprofile-generate=, -fprofile-instr-generate, -fprofile-instr-generate=. I would like to keep line 7-10 in order to verify the change to using “hasFlag” instead of “hasArg”. For the other switches, I could simplify the tests to only check “–fx” and “–fno-x” in order to cut down on test proliferation.
Dec 14 2015
Following Vedant's comments, two test issues have been fixed.
Thanks for reviewing the patch. I try to answer your questions, could you please let me know if this makes sense to you?
Dec 11 2015
Dec 4 2015
Aug 27 2015
Thanks for helping me review and submit the patch.
Aug 20 2015