Add common code path for test configs #491
Conversation
| test('objective_c', () { | ||
| final pubspecFile = File('example/objective_c/pubspec.yaml'); | ||
| final pubspecYaml = loadYaml(pubspecFile.readAsStringSync()) as YamlMap; | ||
| final config = Config.fromYaml(pubspecYaml['ffigen'] as YamlMap); |
There was a problem hiding this comment.
This means we have the config in two places now, how are we going to keep that sync? These tests are here to ensure our examples keep working.
What is the reason for moving the config here instead of reading it from the file?
There was a problem hiding this comment.
Yes, I brought this up with Liam as a potential point of contention offline and we discussed a solution. My proposed solution is to make an additional test helper for reading a config from a file given its path from the project root and to use separate config files instead of a key in pubspec.yaml, so that the config file helper can simply read the file into a string and then use the existing helper, rather than parsing the pubspec.yaml to get the ffigen key
| ${strings.entryPoints}: | ||
| - '/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/AVFAudio.framework/Headers/AVAudioPlayer.h' | ||
| ${strings.preamble}: | | ||
| // ignore_for_file: camel_case_types, non_constant_identifier_names, unused_element, unused_field, return_of_invalid_type, void_checks, annotate_overrides, no_leading_underscores_for_local_identifiers, library_private_types_in_public_api |
There was a problem hiding this comment.
Nit: prefer ignore_for_file on separate lines and alphabetically ordered.
There was a problem hiding this comment.
I believe this was copied verbatim from the existing pubspec.yaml. see my previous comment, though, about how I plan to make this easier to synchronize by using a standalone config.yaml
There was a problem hiding this comment.
Please fix the place where this is copied from while your at it. 🙏
Great! Thanks @TzviPM !
liamappelbe
left a comment
liamappelbe
left a comment
There was a problem hiding this comment.
Thanks for cleaning up!
| // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
| // THE SOFTWARE. | ||
| // ignore_for_file: camel_case_types, non_constant_identifier_names No newline at end of file |
| entry-points: | ||
| - 'headers/example.h' | ||
| preamble: | | ||
| // ignore_for_file: deprecated_member_use No newline at end of file |
| output: 'generated_bindings.dart' | ||
| headers: | ||
| entry-points: | ||
| - 'headers/example.h' No newline at end of file |
| // ignore_for_file: unused_element, unused_field, return_of_invalid_type | ||
| // ignore_for_file: void_checks, annotate_overrides | ||
| // ignore_for_file: no_leading_underscores_for_local_identifiers | ||
| // ignore_for_file: library_private_types_in_public_api No newline at end of file |
|
Oh, looks like a few tests are failing |
|
@TzviPM is this still needed for your work? |
| @@ -1,4 +1,4 @@ | |||
| // Generated by Apple Swift version 5.5.2 (swiftlang-1300.0.40.106 clang-1300.0.29.21) | |||
| // Generated by Apple Swift version 5.7.2 (swiftlang-5.7.2.135.5 clang-1400.0.29.51) | |||
There was a problem hiding this comment.
We haven't pinned clang on the CI?
| // ignore_for_file: unused_element, unused_field, return_of_invalid_type | ||
| // ignore_for_file: void_checks, annotate_overrides | ||
| // ignore_for_file: no_leading_underscores_for_local_identifiers | ||
| // ignore_for_file: library_private_types_in_public_api No newline at end of file |
There was a problem hiding this comment.
nit: newline at end of file
| output: 'generated_bindings.dart' | ||
| headers: | ||
| entry-points: | ||
| - 'headers/example.h' No newline at end of file |
There was a problem hiding this comment.
nit: newline at end of file
3 participants
Fixes dart-lang/native#316.