-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[shared_preferences] Add shared preferences devtools #6749
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
Changes from 1 commit
9a98dce
f6a1ce9
b9c0b6b
e2f1478
b798091
0115bea
421d981
bf23f70
2402e78
215d551
18ac82b
16f30fc
c14409e
382f612
aba50c5
a118924
0604550
eb6dc64
c9dab29
1627ec8
c7c3eb5
bb58727
ee1d2f4
01d7f5c
39b3990
00e568c
21202e6
e489086
c32457f
c6c8068
c4be218
862938b
84c6103
866ef50
7b1a151
bffbe95
d8af525
26f186b
8b005d9
2dc2a33
5b7cbbf
c497026
bc887de
ffd64a9
e95d285
ed56a14
8936da6
966d0fc
3267c1b
1acebd4
3846f73
6eb1b4f
d3fea9e
af0dfca
107fc82
4b2f619
6abec8a
a45bc1c
c1a9c85
612e118
ea80fce
1ef04f6
4df0393
2f8e7ea
b77893c
7a3ec34
e9b913c
8362269
119df62
38954ba
e3c39f2
9f7cdd4
026b225
659b9cd
7bd20c1
d672329
e6da5b8
3c5cecd
c761404
2b7d31f
c80795a
bb0b9be
4f6e93b
4f2d2fd
9838731
a783959
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kenzieschmoll I have an idea on how to create an integration test for this class, but I'm stuck on a step and would like to know if you have any tips. Basically, I want to do the following:
I'm actually stuck on the part where I'd use Here is a code example of what I want to do: void main() {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();
group('SharedPreferencesToolEval integration', () {
testWidgets('should work', (tester) async {
/// Just so dtdManager and serviceManager get initialized
await tester.pumpWidget(
DevToolsExtension(
child: Container(),
),
);
// This is the issue, I can't find a way to get the
// debug session uri programmatically.
final info = await Service.getInfo();
final uri = info.serverUri!;
await dtdManager.connect(uri);
final EvalOnDartLibrary asyncEval = EvalOnDartLibrary(
'package:shared_preferences/src/shared_preferences_async.dart',
serviceManager.service!,
serviceManager: serviceManager,
);
final EvalOnDartLibrary legacyEval = EvalOnDartLibrary(
'package:shared_preferences/src/shared_preferences_legacy.dart',
serviceManager.service!,
serviceManager: serviceManager,
);
final SharedPreferencesToolEval toolEval = SharedPreferencesToolEval(
asyncEval,
legacyEval,
serviceManager.connectedApp?.isFlutterWebAppNow ?? false,
);
// Do the testing here
});
});
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I knew the debug session uri I could even do a normal connection using the UI (driving the test to type the url and click the connect button), so I can call updateVmServiceConnection.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are a couple of issues here:
Right now, this behavior is going to be quite difficult to test, but this is something I'd like to improve for the DevTools extensions framework in general because testing compatibility with the parent package is an important use case. That being said, I recommend following the advice from this comment to test the evaluated expressions, and we can add full integration testing later when that is better supported.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, but if we go with this approach of editing the It would be way easier and we'd make sure that the contract is working, since the I was deliberately not making changes in With this approach, we could just have the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we could even remove this hack, since it would be possible to use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. Well, we can go with this idea, but this PR was kind already done and tested. If we want to go with the Do you think this issue will be prioritized soon? With integration tests I think we should keep the current eval solution, since it would keep the shared_preferences package simpler.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No I don't think this will be prioritized soon with our current resourcing and priorities. That being said, I'm okay with implementing the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stuartmorgan will the publish of
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, any version change will trigger publishing.
It's possible, yes. This PR would have to:
And then no fixes to Generally we only use this for batching breaking changes in a singe major version bump, because blocking all other changes on something unrelated is the opposite of how we generally want our release process to work. I'm not sure I follow why it would be needed here. If we think there is a change we should make to make what this PR is doing a) simpler and b) safer, why wouldn't we make that change before landing the PR?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, we can do on this PR then. I'll work on the |
Uh oh!
There was an error while loading. Please reload this page.