Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generated mocks in test target missing @testable annotation against main target #3353

Closed
michaelloo opened this issue Mar 14, 2024 · 8 comments · Fixed by apollographql/apollo-ios-dev#362
Labels
bug Generally incorrect behavior codegen Issues related to or arising from code generation

Comments

@michaelloo
Copy link

Summary

At work, we recently updated Apollo from 0.52.0 to 1.9.0 (I know thats a fairly big jump!) via Cocoapods.

We used to be able to create test data for the generated GraphQL models using the initialiser, but as referred to in the migration document, we are meant to now use Mocks and ApolloTestSupport instead.

I'd like to generate the tests in the test target since that is where they are needed, they do not need to be shipped in the framework I am packaging but doing so does not compile as the generated files are not public and the generated mocks cannot find them.

Version

1.9.0

Steps to reproduce the behavior

Project setup:

  • Xcode: 15.2
  • Cocoapods: 1.15.2
  • Main target: Framework
  • Dependency management tool: Cocoapods

Example of Podfile setup (as I cannot share work related code):

target 'Foo' do
  pod 'Apollo, '1.9.0'

  target 'FooTests' do
    pod 'ApolloTestSupport', '1.9.0'
  end

Things I've tried:

  1. If I generate the mocks in the test target, whilst they import the main target (where the generated GraphQL files are), they are missing the @testable annotation to find the internal generated GraphQL files.
  2. I could make the generated GraphQL files public, but that means exposing my data models to any other codebase that uses the framework, which isn't ideal
  3. I cannot generate the mocks inside the main target (as per instructions in the migration doc) because it tries to import the main target (and as mentioned, i'd like to avoid shipping them in the framework if possible)

I feel that the output.testMocks code generation should either:

  1. Have the ability to specify @testable if we are adding this to a test target so that we dont need to declare things public for the sake of tests, or
  2. Allow us to use it in the main target because we might also need mocks for things like Xcode Previews

Any other options would be greatly appreciated!

Logs

No response

Anything else?

No response

@michaelloo michaelloo added bug Generally incorrect behavior needs investigation labels Mar 14, 2024
@calvincestari
Copy link
Member

We used to be able to create test data for the generated GraphQL models using the initialiser, but as referred to in the migration document, we are meant to now use Mocks and ApolloTestSupport instead.

I think the selection set initializers config option was built after the migration guides. That may be what you're looking for?

@calvincestari calvincestari added awaiting response and removed bug Generally incorrect behavior needs investigation labels Mar 14, 2024
@michaelloo
Copy link
Author

Thank you @calvincestari ! That worked 👍

Whilst it fixes my problem, I wonder if we still would want the ability to specify whether the import of the generated mocks should have @testable in cases where we dont the mocked models to be public?

Using the Mocks would actually be beneficial but cannot be used without public graphql models.

@calvincestari
Copy link
Member

@michaelloo do you mind sharing your codegen configuration so I can see how you're putting things together.

@AnthonyMDev
Copy link
Contributor

Yeah, I do agree with you @michaelloo. I'm thinking we can determine if the generated models will be public from the configuration and just know to add @testable or not without needing another new config option.

@michaelloo
Copy link
Author

michaelloo commented Mar 14, 2024

@calvincestari here is my latest config (with some obscured params):

{
  "schemaNamespace" : "XXXSchema",
  "schemaDownload": {
    "downloadMethod": {
        "introspection": {
            "endpointURL": "some.url/graphql",
            "httpMethod": { "POST": {} },
            "includeDeprecatedInputValues": false,
            "outputFormat": "JSON"
        }
    },
    "downloadTimeout": 60,
    "outputPath": "./graphql/"
  },
  "input" : {
    "operationSearchPaths" : ["**/*.graphql"],
    "schemaSearchPaths" : ["**/schema.json"]
  },
  "output" : {
    "testMocks" : { "none" : {} },
    "schemaTypes" : {
      "path" : "./GeneratedFiles",
      "moduleType" : { "embeddedInTarget" : { "name": "XXX" } }
    },
    "operations" : { "inSchemaModule" : {} }
  },
  "options": {
    "cocoapodsCompatibleImportStatements": true,
    "selectionSetInitializers": {
      "localCacheMutations" : true,
      "namedFragments" : true,
      "operations" : true
    }
  }
}

I had "testMocks" with "absolute" configuration in there when trying the Mock out


@AnthonyMDev that could work if that is the assumption we take that if the accessModifier for the GQL models are not public, then the mocks are purely for testing. That will couple them together more but might not be a bad thing I guess.

Is adding another AccessModifier case called testable that we can pass to the existing testMocks.absolute config? Although this might also not be ideal because it means you could actually create GQL models with @testable as well and that might not make any sense 🤔

@calvincestari
Copy link
Member

Is adding another AccessModifier case called testable that we can pass to the existing testMocks.absolute config? Although this might also not be ideal because it means you could actually create GQL models with @testable as well and that might not make any sense 🤔

TestMockFileOutput and SchemaTypesFileOutput.ModuleType share the same enum so I don't think an internal case will work there and it is worth duplication of the enum when we can work with assumptions instead?

@AnthonyMDev that could work if that is the assumption we take that if the accessModifier for the GQL models are not public, then the mocks are purely for testing.

Our view is that test mocks are purely for testing; that was the intent of their design. There is also a future where test mocks are deprecated.

@calvincestari calvincestari added bug Generally incorrect behavior codegen Issues related to or arising from code generation and removed awaiting response labels Mar 15, 2024
@calvincestari calvincestari added this to the Patch Releases (1.x.x) milestone Mar 15, 2024
@AnthonyMDev
Copy link
Contributor

This should be fixed by #362 which will be released within the next week.

If you are still experiencing this bug once that is released, please let us know!

Copy link
Contributor

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Generally incorrect behavior codegen Issues related to or arising from code generation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants