Add skylib tests for compile_commands.bzl - #284
Conversation
c64176d to
70913e9
Compare
Szelethus
left a comment
Szelethus
left a comment
There was a problem hiding this comment.
Thats quite a PR! I reviewed it to the best of my ability. I'd like for you to create an issue for each of the observed bugs (or locate them). Plus points if you also mention the issue number in the TODO comments.
| asserts.true( | ||
| env, | ||
| "IMPL_DEP_LOCAL_DEF" in foo_commands[0], | ||
| "Should contain local_define IMPL_DEP_LOCAL_DEF from implementation_dep, got: %s" % foo_commands[0], |
Add qoute include test Remove most tests Undo refactors for compile_commands.bzl
70913e9 to
0c149bd
Compare
Co-authored-by: Kristóf Umann <dkszelethus@gmail.com>
| @@ -0,0 +1,423 @@ | |||
| # Copyright 2026 Ericsson AB | |||
There was a problem hiding this comment.
Suggestion: name file like test/unit/compile_commands/analysis_test.bzl instead of test/unit/compile_commands/compile_commands_analysis_test.bzl
No need to duplicate "compile_commands".
| get_compile_flags_defines_test( | ||
| name = name + "_defines_test", | ||
| target_under_test = ":" + name + "_with_defines", | ||
| ) | ||
| get_compile_flags_local_defines_test( | ||
| name = name + "_local_defines_test", | ||
| target_under_test = ":" + name + "_with_local_defines", | ||
| ) | ||
| get_compile_flags_includes_test( | ||
| name = name + "_includes_test", | ||
| target_under_test = ":" + name + "_with_includes", | ||
| ) | ||
| get_compile_flags_copts_test( | ||
| name = name + "_copts_test", | ||
| target_under_test = ":" + name + "_with_copts", | ||
| ) | ||
| get_compile_flags_dep_includes_test( | ||
| name = name + "_dep_includes_test", | ||
| target_under_test = ":" + name + "_with_dep_includes", | ||
| ) | ||
| get_compile_flags_system_includes_test( | ||
| name = name + "_system_includes_test", | ||
| target_under_test = ":" + name + "_with_system_includes", | ||
| ) | ||
| get_compile_flags_quote_includes_test( |
There was a problem hiding this comment.
These test names are just insane!
It is very hard to find which one to debug :)
| # Split command into flags and check for duplicates | ||
| flags = foo_commands[0].split(" ") | ||
| seen = [] | ||
| duplicates = [] | ||
| for f in flags: | ||
| if f == "": | ||
| continue | ||
| if f in seen and f not in duplicates: | ||
| duplicates.append(f) | ||
| seen.append(f) |
There was a problem hiding this comment.
This will produce wrong list: splitting by space will give many "-I" and "-iquote" with their values separately - not that kind of duplication :)
Well, I guess the way how issues were created it kind of destroyed the idea of reporting these as bugs :) |
3 participants
Why:
We want to properly test compile_commands generation.
What:
Addresses:
none