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

fix: Clarify setup guide #64

Open
1 task done
FufferKS opened this issue Jun 13, 2022 · 3 comments
Open
1 task done

fix: Clarify setup guide #64

FufferKS opened this issue Jun 13, 2022 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@FufferKS
Copy link

Is there an existing issue for this?

  • I have searched the existing issues.

Version

alchemist: ^0.3.3

Description

Hi, great package, solves an important problem! However, I am not sure two aspects are clear.

  1. The setup guide suggests checking for environment variables in a different fashion the what is used in the example. For me, only the example way works, perhaps it's worth it update it?

  2. I am not sure I understand how you are handling the platform tests across developers. Since platform goldens are not tracked in CI, what happens when someone adds a new test? Wouldn't I have to first always run flutter test --update-goldens on my local machine?

Steps to reproduce

Expected behavior

The setup guide, especially in terms of handling the platform tests, should be clear.

Screenshots

No response

Additional context and comments

No response

@FufferKS FufferKS added the bug Something isn't working label Jun 13, 2022
@jeroen-meijer jeroen-meijer self-assigned this Jun 14, 2022
@jeroen-meijer
Copy link
Collaborator

Hi there,

Thanks for opening up an issue! You raise some great points.

1)

The recommended setup guide is used to illustrate how we used Alchemist in our workflow. The example could use a similar configuration to determine whether it's being executed on CI or otherwise. This part has been left out of the example for both simplicity's sake and since the example (currently) isn't being tested.

If you think it'd be valuable to add this environment mechanic to the example, I'm open to revising this :)

2)

Correct, I've recently noticed the same things. Since platform goldens are generally not tracked in source control, Flutter will throw an error when trying to run goldens on any fresh copy of a repo on a local machine.

We have a couple options to accommodate for this. One is to add better documentation around this mechanic to make sure new users are aware of this discrepancy.
Another interesting proposal would be to add a generateMissingPlatformGoldens flag to AlchemistConfig (or generateMissingGoldens on PlatformGoldensConfig) that, when enabled, makes sure missing platform golden files are generated and their checks passed automatically.

Pinging @Kirpal and @definitelyokay for their opinion on this, especially the second point.

@Kirpal
Copy link
Collaborator

Kirpal commented Jun 14, 2022

I like that idea! I lean towards having generateMissingGoldens as a parameter on the abstract GoldensConfig class, so it could be applied to the CiGoldensConfig as well, just in case someone wanted that.

@FufferKS
Copy link
Author

Hi, thanks for getting back to me.

  1. I am not sure generateMissingGoldens is enough. A simple check for a missing file will help for the new tests, but it won’t be enough if the underlaying component and ci golden is updated, since we end up with a stale platform golden.

I don’t have better idea at this moment, will try to give it more thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants