[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-server/…
2023-02-05 03635, 2023
ArjunM joined the channel
2023-02-05 03604, 2023
serialata has quit
2023-02-05 03634, 2023
jasje joined the channel
2023-02-05 03610, 2023
jasje
akshaaatt: ping again
2023-02-05 03603, 2023
jasje has quit
2023-02-05 03625, 2023
jasje joined the channel
2023-02-05 03644, 2023
bosonbear has quit
2023-02-05 03658, 2023
jasje has quit
2023-02-05 03643, 2023
bosonbear joined the channel
2023-02-05 03611, 2023
bosonbear has quit
2023-02-05 03645, 2023
ArjunM has quit
2023-02-05 03603, 2023
ArjunM joined the channel
2023-02-05 03603, 2023
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-androi…
2023-02-05 03645, 2023
ArjunM has quit
2023-02-05 03640, 2023
BrainzGit
[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-androi…
2023-02-05 03609, 2023
santiagofn has quit
2023-02-05 03626, 2023
jasje joined the channel
2023-02-05 03643, 2023
jasje
hi akshaaatt
2023-02-05 03628, 2023
s[_] joined the channel
2023-02-05 03620, 2023
akshaaatt
What’s up?
2023-02-05 03637, 2023
akshaaatt
You wanted to discuss something right?
2023-02-05 03621, 2023
jasje
yupp
2023-02-05 03653, 2023
jasje
akshaaatt: about the app architecture
2023-02-05 03616, 2023
akshaaatt
Tell me
2023-02-05 03633, 2023
jasje
so when glazing through most of the app
2023-02-05 03649, 2023
jasje
stuff isn’t done as per the architecture we ise
2023-02-05 03614, 2023
jasje
plenty stuff done inside activities composables
2023-02-05 03630, 2023
jasje
i also kinda realise why we shouldn’t use context
2023-02-05 03658, 2023
jasje
We just can’t conduct ui tests unless we abstract the whole app as per the architecture
2023-02-05 03623, 2023
jasje
The main enemy right now is sharedPreferences
2023-02-05 03632, 2023
jasje
why you may ask
2023-02-05 03618, 2023
jasje
because it uses context which we can’t mock if we want to make tests dependent more on the codebase itself
2023-02-05 03629, 2023
jasje
now this line might seem confusing
2023-02-05 03635, 2023
jasje
so lemme explain
2023-02-05 03602, 2023
jasje
usually i may test a composable by just declaring it
2023-02-05 03623, 2023
akshaaatt
Let’s go step by step
2023-02-05 03631, 2023
jasje
testRule.setContent { MyComposable() }
2023-02-05 03640, 2023
jasje
yuss
2023-02-05 03642, 2023
akshaaatt
What do you mean by “testing a composable”?
2023-02-05 03651, 2023
jasje
lemme login from from my pc hang on
2023-02-05 03606, 2023
akshaaatt
I can already see the preview, I don’t need any more tests for the UI of the composable
2023-02-05 03647, 2023
jasje_ joined the channel
2023-02-05 03628, 2023
jasje_
no no
2023-02-05 03646, 2023
akshaaatt
Go on
2023-02-05 03625, 2023
jasje_
There's two ways we can test a screen (UI)
2023-02-05 03654, 2023
jasje_
1) We just start the activity which is independent of external factors
2023-02-05 03657, 2023
jasje_
or
2023-02-05 03640, 2023
jasje_
2) We define limited contents of the activity inside our test and set the content of our activitys navitgation or composable
2023-02-05 03647, 2023
akshaaatt
An app can’t be independent of stuff. Only components can be, but an activity can’t/shouldn’t be independent
2023-02-05 03648, 2023
jasje_
like viewmodels
2023-02-05 03601, 2023
jasje_
yupp
2023-02-05 03621, 2023
jasje_
thats why i am going for the second approach\
2023-02-05 03621, 2023
akshaaatt
2 sounds good to me
2023-02-05 03634, 2023
jasje_
so
2023-02-05 03640, 2023
jasje_
the problem here will be
2023-02-05 03626, 2023
jasje_
if we just let our viewmodel run itll run into errors because tests are offline and some components should be mocked
2023-02-05 03635, 2023
jasje_
so
2023-02-05 03651, 2023
jasje_
we mock those components and instantiate the viewmodels
2023-02-05 03654, 2023
jasje_
but still
2023-02-05 03655, 2023
akshaaatt
Shouldn’t we use our mock viewmodels for these UI tests?
2023-02-05 03602, 2023
akshaaatt
Right
2023-02-05 03626, 2023
jasje_
No viewmodels shouldn't be mocked until necessary because
2023-02-05 03644, 2023
jasje_
any change in the codebase then must be edited again in the mock
2023-02-05 03603, 2023
akshaaatt
Think of it this way
2023-02-05 03603, 2023
jasje_
that is not good practice but i wont say its not necessary
2023-02-05 03654, 2023
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
2023-02-05 03618, 2023
akshaaatt
I need to test this navigation through my UI tests
2023-02-05 03645, 2023
akshaaatt
I will have to make a mock viewmodel in this case because I won’t be able to make my request, right?
2023-02-05 03656, 2023
jasje_
no
2023-02-05 03609, 2023
jasje_
mocking the repository is enough for this
2023-02-05 03615, 2023
akshaaatt
Alright
2023-02-05 03617, 2023
akshaaatt
Sounds good
2023-02-05 03637, 2023
jasje_
so now we are at viewmodel(mockrepo())
2023-02-05 03641, 2023
akshaaatt
Also, it just clicked to me that these UI tests will run on emulators, right?
2023-02-05 03647, 2023
jasje_
yes
2023-02-05 03647, 2023
akshaaatt
Yes jasje_
2023-02-05 03654, 2023
jasje_
so
2023-02-05 03609, 2023
jasje_
now another problem still persists
2023-02-05 03618, 2023
jasje_
as i mentioned
2023-02-05 03624, 2023
jasje_
sharedPreferences
2023-02-05 03624, 2023
akshaaatt
Sounds good
2023-02-05 03643, 2023
jasje_
sharedPreferences require context
2023-02-05 03647, 2023
akshaaatt
Yes
2023-02-05 03609, 2023
jasje_
our Preferences object uses app.context which causes the app to crash
2023-02-05 03615, 2023
jasje_
what i propose is
2023-02-05 03638, 2023
akshaaatt
I’m pretty sure most of the apps use shared preferences. We can’t/shouldn’t get rid of them
2023-02-05 03642, 2023
jasje_
since sharedPreferences are also "Single source of truth" as mentioned in the docs
2023-02-05 03656, 2023
akshaaatt
What’s the solution then because we are not removing those
2023-02-05 03602, 2023
jasje_
we move them to repository that we then inject to our viewmodel
2023-02-05 03619, 2023
akshaaatt
Hmmm. That sounds doable to me
2023-02-05 03622, 2023
jasje_
now you may ask
2023-02-05 03629, 2023
jasje_
how does this help?
2023-02-05 03641, 2023
akshaaatt
No I understand how it would help
2023-02-05 03642, 2023
jasje_
We cant mock context
2023-02-05 03653, 2023
jasje_
bbut we can mock repos
2023-02-05 03603, 2023
jasje_
also another benefit is that
2023-02-05 03649, 2023
jasje_
sharedPreferences would be collected in a good place that i dont need to import every now and then
2023-02-05 03604, 2023
akshaaatt
Fair enough
2023-02-05 03621, 2023
jasje_
This good so we move next?
2023-02-05 03632, 2023
akshaaatt
Do you any github repos with the relevant architecture in mind to share?
2023-02-05 03638, 2023
akshaaatt
I would like to have a look
2023-02-05 03658, 2023
akshaaatt
Also, I would want us to restructure the codebase with proper folder structure
2023-02-05 03612, 2023
jasje_
Unfortunately i don't have any such repos as of now
2023-02-05 03620, 2023
jasje_
its just me
2023-02-05 03637, 2023
akshaaatt
model
2023-02-05 03659, 2023
jasje_
model as in preview of what it would look like?
2023-02-05 03601, 2023
akshaaatt
Ui -> screens, components, entrypoint
2023-02-05 03611, 2023
akshaaatt
Viewmodel
2023-02-05 03626, 2023
akshaaatt
That’s the folder structure I’m thinking of
2023-02-05 03629, 2023
akshaaatt
Utils
2023-02-05 03655, 2023
akshaaatt
Services
2023-02-05 03659, 2023
akshaaatt
Repository
2023-02-05 03613, 2023
akshaaatt
These 6 should do for us
2023-02-05 03621, 2023
jasje_
di
2023-02-05 03631, 2023
jasje_
important
2023-02-05 03642, 2023
jasje_
our di modules are scrambled right now
2023-02-05 03600, 2023
akshaaatt
Hmmm. iOS doesn’t need libraries like hilt though I wonder
2023-02-05 03614, 2023
akshaaatt
I was just reframing the arc I use for iOS in general
2023-02-05 03628, 2023
akshaaatt
di should do for our android repo as wel
2023-02-05 03633, 2023
akshaaatt
7 then
2023-02-05 03640, 2023
jasje_
look look
2023-02-05 03656, 2023
jasje_
for now the structure is good
2023-02-05 03605, 2023
jasje_
most important thing is to
2023-02-05 03621, 2023
jasje_
abstract sharedPrefs
2023-02-05 03623, 2023
jasje_
then
2023-02-05 03654, 2023
jasje_
create viewmodels for some features i was glazing through (best convert to compose)
2023-02-05 03611, 2023
akshaaatt
No jasje_ the structure currently isn’t too clear because we have the ui and viewmodels all under the features folder
2023-02-05 03649, 2023
akshaaatt
Even the data folder is messed up
2023-02-05 03602, 2023
akshaaatt
Only a few people can understand the codebase rn
2023-02-05 03629, 2023
akshaaatt
Converting the viewmodels to compose you mean jasje_ ? That doesn’t make sense
2023-02-05 03635, 2023
jasje_
no no
2023-02-05 03643, 2023
jasje_
the xmls to compose
2023-02-05 03600, 2023
akshaaatt
Aligned on that
2023-02-05 03617, 2023
jasje_
also
2023-02-05 03607, 2023
jasje_
could you get the new guy to create a component of caching service?
2023-02-05 03642, 2023
jasje_
which also includes auto cleanup
2023-02-05 03610, 2023
jasje_
or we could just use a third party lib that does just that
2023-02-05 03644, 2023
akshaaatt
Sure we should ask him
2023-02-05 03612, 2023
akshaaatt
If you have any good third party libs in mind, let me know jasje_
