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: export StartOptions and MobileScannerViewAttributes for tests #1312

Merged
merged 3 commits into from
Feb 3, 2025

Conversation

w3ggy
Copy link

@w3ggy w3ggy commented Jan 31, 2025

I added StartOptions and MobileScannerViewAttributes to export to let the developers to mock the MobileScannerPlatform in tests.

I tried to write golden tests and mock the MobileScannerPlatform but I cannot do it because I cannot import classes which are necessary to override Future<MobileScannerViewAttributes> start(StartOptions startOptions)

… developers to mock `MobileScannerPlatform`
@navaronbracke
Copy link
Collaborator

navaronbracke commented Feb 1, 2025

@w3ggy Should we mark this as @visibleForTesting ? Otherwise, I think this looks good to have.
While there aren't many people that write tests for mobile_scanner usage, there are definitely some.

See also #1042 which is also related to tests and we should look into.

@navaronbracke navaronbracke changed the title Add StartOptions and MobileScannerViewAttributes to export to let… fix: export StartOptions and MobileScannerViewAttributes for tests Feb 1, 2025
@navaronbracke
Copy link
Collaborator

@w3ggy I went ahead and verified that we do not need @visibleForTesting here, since that allows it to be used only in tests and the defining library. The latter is not the case, as the platform interface definition uses it.

I updated your pull request with a changelog entry and fixed the import order, per the https://dart.dev/tools/linter-rules/directives_ordering rule.

Thanks for catching this!

@navaronbracke navaronbracke merged commit 7a42274 into juliansteenbakker:develop Feb 3, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants