.prefire.yml ignored by PrefirePlaybook plugin

szymon-heetch commented 6 months ago

Context 🕵️‍♀️

Just updated from 1.5.0 to 2.3.0. I use playbook and tests plugins for my target. Configuration file uses - preview_default_enabled: false to manually choose previews for snapshots and playbook view.

.prefire.yml content:

test_configuration:
  - preview_default_enabled: false
  - target: [Redacted]
  - simulator_device: "iPhone15,2"
  - required_os: 16
  - snapshot_devices:
    - iPhone 14 Pro

prefire_configuration:
  - preview_default_enabled: false

What 🌱

Running Prefire Tests plugin: PrefireTests

Running Prefire Playbook plugin: PrefirePlaybook

BarredEwe commented 6 months ago

@szymon-heetch Thanks for starting the issue! I posted a new version with a fix for this: https://github.com/BarredEwe/Prefire/releases/tag/2.3.1

szymon-heetch commented 6 months ago

Thanks for fast reply @BarredEwe! I see some inconsistencies tho. I still use config above. Here's what I observed:

PrefirePlaybookPlugin

`#Preview` macro

preview_default_enabled: false setting is still ignored - all my previews are automatically generated. When I add .prefireIgnored() explicitly, then it's skipped from generation (correct behavior), but I think it should be skipped automatically and .prefireEnabled() should mark previews for generation only.
PrefireProvider protocol conformance
all previews marked with above protocol are correctly generated. I believe setting preview_default_enabled: false only applies to macros (correct me if I'm wrong).

PrefireTestPlugin

Nothing is generated in both cases (#Preview and PrefireProvider)

Running Test plugin with config:

test_configuration:
  ....
  - preview_default_enabled: true

prefire_configuration:
  - preview_default_enabled: false

gets last preview_default_enabled: false setting. test_configuration and prefire_configuration sections are ignored.

My test scenario

Single framework
Use configuration .yml above
Exclusively use PrefirePlaybookPlugin or PrefireTestPlugin (never both at the same time)

BarredEwe commented 6 months ago

There is a problem in the documentation Readme.md . That's not what the playbook configuration is called right now.

test_configuration:
  - preview_default_enabled: true

playbook_configuration:
  - preview_default_enabled: false

You need to use playbook_configuration instead of prefire_configuration

szymon-heetch commented 6 months ago

You need to use playbook_configuration instead of prefire_configuration

Ok, that indeed solves first issue 😅

Still no snapshots tests 🤔

BarredEwe commented 6 months ago

You need to use playbook_configuration instead of prefire_configuration

Ok, that indeed solves first issue 😅

Still no snapshots tests 🤔

Which version of Prefire are you using?

szymon-heetch commented 6 months ago

Updated from 1.5.0 to 2.4.0.

BarredEwe commented 6 months ago

Can you check if the Prefire has identified it correctly:

Prefire configuration
→ Target used for tests: -
→ Tests target: -

szymon-heetch commented 6 months ago

I followed the path to generated sources and PreviewModels.generated.swift doesn't have any models added.

It's reproducible when I use only PrefireTestPlugin in my test target - no Playbook plugin used.

BarredEwe commented 6 months ago

It's reproducible when I use only PrefireTestPlugin in my test target - no Playbook plugin used.

I tried it in different ways, connected only PrefireTestPlugin and so on. I can't reproduce the problem. If you can describe a simple case of how to reproduce this problem, it will help me a lot 😊

szymon-heetch commented 6 months ago

I created sample project which mimics my setup - the lead could be that tests are separated to standalone Framework 🤔

szymon-heetch commented 5 months ago

I dug a bit by myself and found out that list of files passed to sourcery is from app target instead of framework.

sources:
  - /Users/.../prefire-issue/PrefireIssue/PrefireIssue/ContentView.swift
  - /Users/.../prefire-issue/PrefireIssue/PrefireIssue/PrefireIssueApp.swift

EDIT: I think I found the issue. This condition results in first target which differs from tested target - which in sample project case is App target - which at the end results in wrong files passed to Sourcery. I think about changing condition to:

target.displayName.hasPrefix($0.displayName)

@BarredEwe WDYT?

EDIT 2: Or be super explicit and support Frameworks with naming pattern - <FrameworkName> & <FrameworkName>Tests :

$0.displayName + "Tests" == target.displayName

BarredEwe commented 5 months ago

EDIT 2: Or be super explicit and support Frameworks with naming pattern - <FrameworkName> & <FrameworkName>Tests :
$0.displayName + "Tests" == target.displayName

That's a great idea. I think this approach will allow not to access the configuration via yml so often. I'll try to implement it now!

BarredEwe commented 5 months ago

Fixed in release: https://github.com/BarredEwe/Prefire/releases/tag/2.5.0

BarredEwe / Prefire