Details
Diff Detail
Event Timeline
lib/fuzzer/tests/CMakeLists.txt | ||
---|---|---|
29–61 | It would be useful to at least have a comment to that effect. |
test/fuzzer/lit.cfg | ||
---|---|---|
55 | The naming isn't great here. You're using cflags but you're passing them to a C++ compiler. Normally the name cxxflags is used for C++ compiler flags and cflags for C compiler flags. | |
57 | I'm not a fan of the old style python string interpolation. I'd rather you used the .format() method on python strings. | |
79 | I realise that -O2 was there before but why are we passing that. Shouldn't the test case decide the optimization level? Also this looks similar to the ASan/TSan compiler invocation. Could we perhaps do a refactor some common flags shared by all tests types are set in one place? | |
test/fuzzer/merge-posix.test | ||
1 | Any reason for XFAIL instead of UNSUPPORTED? Also for each test that you marked as XFAIL it would nice to have a short comment explaining why its XFAIL for ios. |
test/fuzzer/lit.cfg | ||
---|---|---|
55 | yeah, i guess _flags might make more sense | |
57 | sorry, let's only post comments specific to this change. old-style python string interpolation was there before, change to .format can be done in a separate patch | |
79 | again, let's only discuss changes in this patch. both change to the compiler level, and extracting out the common code is orthogonal. | |
test/fuzzer/merge-posix.test | ||
1 | I think we don't do ulimit properly for device testing yet. My goal is to bring up something workable fast, and then further changes might be incrementally made to de-XFAILify more tests |
Is the intent to skip ARM64 here? It seems like you'd be better off keeping the loop and doing an early continue if ARM64 instead.