[musicbrainz-server] 14mwiencek opened pull request #2822 (03master…mbs-12832): MBS-12832: Improve edit submission batching in the release relationship editor https://github.com/metabrainz/musicbrainz-serve...
ArjunM joined the channel
serialata has quit
jasje joined the channel
jasje
akshaaatt: ping again
jasje has quit
jasje joined the channel
bosonbear has quit
jasje has quit
bosonbear joined the channel
bosonbear has quit
ArjunM has quit
ArjunM joined the channel
BrainzGit
[listenbrainz-android] 14akshaaatt merged pull request #46 (03main…main): Feat: Added recently played music and feature to add favourites from music player. https://github.com/metabrainz/listenbrainz-andr...
ArjunM has quit
[listenbrainz-android] 14dependabot[bot] closed pull request #37 (03main…dependabot/gradle/room_version-2.5.0): Bump room_version from 2.4.3 to 2.5.0 https://github.com/metabrainz/listenbrainz-andr...
santiagofn has quit
jasje joined the channel
jasje
hi akshaaatt
s[_] joined the channel
akshaaatt
What’s up?
You wanted to discuss something right?
jasje
yupp
akshaaatt: about the app architecture
akshaaatt
Tell me
jasje
so when glazing through most of the app
stuff isn’t done as per the architecture we ise
plenty stuff done inside activities composables
i also kinda realise why we shouldn’t use context
We just can’t conduct ui tests unless we abstract the whole app as per the architecture
The main enemy right now is sharedPreferences
why you may ask
because it uses context which we can’t mock if we want to make tests dependent more on the codebase itself
now this line might seem confusing
so lemme explain
usually i may test a composable by just declaring it
akshaaatt
Let’s go step by step
jasje
testRule.setContent { MyComposable() }
yuss
akshaaatt
What do you mean by “testing a composable”?
jasje
lemme login from from my pc hang on
akshaaatt
I can already see the preview, I don’t need any more tests for the UI of the composable
jasje_ joined the channel
jasje_
no no
akshaaatt
Go on
jasje_
There's two ways we can test a screen (UI)
1) We just start the activity which is independent of external factors
or
2) We define limited contents of the activity inside our test and set the content of our activitys navitgation or composable
akshaaatt
An app can’t be independent of stuff. Only components can be, but an activity can’t/shouldn’t be independent
jasje_
like viewmodels
yupp
thats why i am going for the second approach\
akshaaatt
2 sounds good to me
jasje_
so
the problem here will be
if we just let our viewmodel run itll run into errors because tests are offline and some components should be mocked
so
we mock those components and instantiate the viewmodels
but still
akshaaatt
Shouldn’t we use our mock viewmodels for these UI tests?
Right
jasje_
No viewmodels shouldn't be mocked until necessary because
any change in the codebase then must be edited again in the mock
akshaaatt
Think of it this way
jasje_
that is not good practice but i wont say its not necessary
akshaaatt
I have a button in my UI, which on clicked, makes a POST request using retrofit and returns me back a response, based on which, I decide whether to navigate that user to screen A or screen B
I need to test this navigation through my UI tests
I will have to make a mock viewmodel in this case because I won’t be able to make my request, right?
jasje_
no
mocking the repository is enough for this
akshaaatt
Alright
Sounds good
jasje_
so now we are at viewmodel(mockrepo())
akshaaatt
Also, it just clicked to me that these UI tests will run on emulators, right?
jasje_
yes
akshaaatt
Yes jasje_
jasje_
so
now another problem still persists
as i mentioned
sharedPreferences
akshaaatt
Sounds good
jasje_
sharedPreferences require context
akshaaatt
Yes
jasje_
our Preferences object uses app.context which causes the app to crash
what i propose is
akshaaatt
I’m pretty sure most of the apps use shared preferences. We can’t/shouldn’t get rid of them
jasje_
since sharedPreferences are also "Single source of truth" as mentioned in the docs
akshaaatt
What’s the solution then because we are not removing those
jasje_
we move them to repository that we then inject to our viewmodel
akshaaatt
Hmmm. That sounds doable to me
jasje_
now you may ask
how does this help?
akshaaatt
No I understand how it would help
jasje_
We cant mock context
bbut we can mock repos
also another benefit is that
sharedPreferences would be collected in a good place that i dont need to import every now and then
akshaaatt
Fair enough
jasje_
This good so we move next?
akshaaatt
Do you any github repos with the relevant architecture in mind to share?
I would like to have a look
Also, I would want us to restructure the codebase with proper folder structure
jasje_
Unfortunately i don't have any such repos as of now
its just me
akshaaatt
model
jasje_
model as in preview of what it would look like?
akshaaatt
Ui -> screens, components, entrypoint
Viewmodel
That’s the folder structure I’m thinking of
Utils
Services
Repository
These 6 should do for us
jasje_
di
important
our di modules are scrambled right now
akshaaatt
Hmmm. iOS doesn’t need libraries like hilt though I wonder
I was just reframing the arc I use for iOS in general
di should do for our android repo as wel
7 then
jasje_
look look
for now the structure is good
most important thing is to
abstract sharedPrefs
then
create viewmodels for some features i was glazing through (best convert to compose)
akshaaatt
No jasje_ the structure currently isn’t too clear because we have the ui and viewmodels all under the features folder
Even the data folder is messed up
Only a few people can understand the codebase rn
Converting the viewmodels to compose you mean jasje_ ? That doesn’t make sense
jasje_
no no
the xmls to compose
akshaaatt
Aligned on that
jasje_
also
could you get the new guy to create a component of caching service?
which also includes auto cleanup
or we could just use a third party lib that does just that
akshaaatt
Sure we should ask him
If you have any good third party libs in mind, let me know jasje_