First compiler contribution - question about testing

Development Compiler
1

Hi all,

I'm looking to get more involved with Swift compiler development. My first contribution is a fix addressing a FIXME issue in swift::isPlatformActive method. (PlatformKind.cpp) My branch is here.

I'd like to add a test for this (seems like a unit test is sufficient) but I'm not sure what the right place for it is. I don't see any other tests or unittests that touch PlatformKind. Would a new unit test suite be appropriate to create here?

Thanks for your help!

Andrew

1 Like
2

Hey, Andrew. Welcome to the project!

The tests in unittests/AST/ do seem like a good fit for this kind of change: a self-contained data structure that can be tested directly. If none of the files there seem like a good place to put the new test, it's fine to make a new one and name it after what you're testing. (Maybe AvailabilityTests.cpp?) You'll have to add it to the CMakeLists.txt file in that folder to get it to compile, too.

That said, I don't think your change is the improvement the FIXME is looking for. It's not about using a switch; it's that the application-extension platform variants don't have a good mapping to the non-application-extension ones.

I suggest taking a look at the issues tagged StarterBug on https://bugs.swift.org. They're a little bigger than just a rewrite like this (okay, some of them are quite a bit bigger), but they're also places where people who work on the compiler regularly already have a solution in mind and can probably help you out.

1 Like
3

Oh, one more note: even just writing tests for things that aren't tested is appreciated, so you could still add a new unit test for PlatformKind and verify that it's doing sensible things with today's implementation. :-)

1 Like
4

Hey Jordan, thank you for the warm welcome and the feedback!

I'm happy to write some tests. This class makes use of LangOptions class to figure out what to return, I'm wondering if there's a simple way to mock it for unit testing purposes or should I construct a specific instance for each platform type in each test instead? Thanks again for the guidance!

Andrew

5

LangOptions is mostly just a bag of values, so constructing an instance and filling out the relevant fields for a test seems totally reasonable to me!