Skip to content

Conversation

@ezaquarii
Copy link
Collaborator

@ezaquarii ezaquarii commented Jun 10, 2019

If you are wondering where ETM (Engineering Test Mode) term came from, it's automotive industry. If you have a car, you can probably play with it there too. :)

This module allows developers and dev users to access some internal application data.
The module is indended to be a dumping ground for various diagnostic and development tools.

  1. Go to Settings
  2. Scroll to section "Dev" (bottom)
  3. Select "Engineering Test Mode"

Signed-off-by: Chris Narkiewicz hello@ezaquarii.com

@ezaquarii
Copy link
Collaborator Author

@tobiasKaminsky @AndyScherzinger I created this PR to help a user with #3421 that was unable to share relevant diagnostics with us.

Please review and comment. It's WIP as I don't want to invest more time into it before we make sure it's worth it.

@christianlupus
Copy link

@ezaquarii Now things are gettign strange. The app is installed parallel to the main app. With the QA version, I can open the app without swipe code even if I set it up like this in the settings.I tried to quit the app by going into the task manager and there swiping out the app. Still the app opens directly without protection, content is shown in the task switcher and I can make screenshots.

So unfortunately this branch is not working as intended as a debug tool....

@ezaquarii
Copy link
Collaborator Author

@christianlupus You are right that this "QA" version is installed as a separate app. QA versions are intended for evaluation without overwriting your daily-driver app, especially that it might not work or cause data corruption. This way your data is safe and the experiment is done in a "sandbox".

Regarding the locking mechanism - you have probably found another bug. We'll be investigating it when @tobiasKaminsky comes back.

@ezaquarii
Copy link
Collaborator Author

@tobiasKaminsky Let me know what you think about it - I don't want to invest more time into it if it's a bad idea. (-:

@tobiasKaminsky
Copy link
Member

@tobiasKaminsky Let me know what you think about it - I don't want to invest more time into it if it's a bad idea. (-:

If we put this for now only on dev/daily version, I am fine with it.
So if an user experiences a bug where we need more infos, we can ask him to test dev version (which then is based on master, where the problem is maybe already fixed) and additional can ask them for infos via ETM.

Big question would be, which infos do we want to have collected…

@ezaquarii
Copy link
Collaborator Author

Big question would be, which infos do we want to have collected…

Well, when you browse the backlog of bugs, it is quite clear that we're lacking audit capabilities as there is no way of getting meaningful data from users currently. My intention was to create a dumping ground for such tools, as I needed to investigate preferences relevant to #3421 and there was no tool to do it.

I'm sure the tech support would love to have it.

Quite frankly, if you could reach them and ask them for feature requests, that would be great. I'm sure they are having tons of interesting stories to tell...

@ezaquarii ezaquarii force-pushed the ezaquarii/engineering-test-mode branch from 8e20f00 to ce69993 Compare July 9, 2019 22:44
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
@nextcloud nextcloud deleted a comment Jul 9, 2019
Copy link
Member

@tobiasKaminsky tobiasKaminsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see inline comments

@ezaquarii ezaquarii force-pushed the ezaquarii/engineering-test-mode branch 2 times, most recently from c93d781 to 034cf50 Compare July 23, 2019 21:07
@codecov
Copy link

codecov bot commented Jul 23, 2019

Codecov Report

Merging #4133 into master will decrease coverage by <.01%.
The diff coverage is 15.87%.

@@             Coverage Diff              @@
##             master    #4133      +/-   ##
============================================
- Coverage     14.33%   14.32%   -0.01%     
  Complexity        1        1              
============================================
  Files           331      340       +9     
  Lines         31030    31156     +126     
  Branches       4405     4423      +18     
============================================
+ Hits           4447     4462      +15     
- Misses        25801    25909     +108     
- Partials        782      785       +3
Impacted Files Coverage Δ Complexity Δ
...java/com/nextcloud/client/di/ComponentsModule.java 0% <ø> (ø) 0 <0> (ø) ⬇️
src/main/java/com/owncloud/android/MainApp.java 54.7% <ø> (ø) 0 <0> (ø) ⬇️
.../main/java/com/nextcloud/client/etm/EtmActivity.kt 0% <0%> (ø) 0 <0> (?)
...n/java/com/nextcloud/client/etm/EtmMenuFragment.kt 0% <0%> (ø) 0 <0> (?)
...n/java/com/nextcloud/client/etm/EtmBaseFragment.kt 0% <0%> (ø) 0 <0> (?)
...owncloud/android/ui/activity/SettingsActivity.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...in/java/com/nextcloud/client/etm/EtmMenuAdapter.kt 0% <0%> (ø) 0 <0> (?)
...xtcloud/client/etm/pages/EtmPreferencesFragment.kt 0% <0%> (ø) 0 <0> (?)
...n/java/com/nextcloud/client/di/ViewModelFactory.kt 0% <0%> (ø) 0 <0> (?)
...in/java/com/nextcloud/client/di/ViewModelModule.kt 0% <0%> (ø) 0 <0> (?)
... and 14 more

@ezaquarii ezaquarii force-pushed the ezaquarii/engineering-test-mode branch from 034cf50 to 4e75f6a Compare July 23, 2019 21:24
@AndyScherzinger
Copy link
Member

@tobiasKaminsky can you re-review. Imho all review commetns have been fixed, I just optimized one padding value for toolbar text alignment, so this is imho ready to be merged now 👍

Nice work @ezaquarii - as always 🎉

@AndyScherzinger
Copy link
Member

All things turned green, just waiting for @tobiasKaminsky's final approval :)

Signed-off-by: Chris Narkiewicz <hello@ezaquarii.com>
@ezaquarii ezaquarii force-pushed the ezaquarii/engineering-test-mode branch from 4f7a04f to 7d3d2c9 Compare July 25, 2019 18:47
@ezaquarii ezaquarii requested a review from tobiasKaminsky July 25, 2019 18:47
@nextcloud-android-bot
Copy link
Collaborator

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/10088.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@nextcloud-android-bot
Copy link
Collaborator

Codacy

281

Lint

TypemasterPR
Warnings5959
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings25
Correctness Warnings70
Internationalization Warnings12
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings120
Security Warnings47
Dodgy code Warnings136
Total423

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings25
Correctness Warnings70
Internationalization Warnings12
Malicious code vulnerability Warnings4
Multithreaded correctness Warnings9
Performance Warnings120
Security Warnings47
Dodgy code Warnings136
Total423

@ezaquarii
Copy link
Collaborator Author

@tobiasKaminsky ping!

@@ -0,0 +1,39 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rename this to etm_layout.xml?
Then we have this in sync to all other names.

@@ -0,0 +1,32 @@
<!--
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe etm_fragment_menu? So that etm is in front

@@ -0,0 +1,33 @@
<!--
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, etm in front?

@tobiasKaminsky
Copy link
Member

Just litte naming ideas: have ETM in front of activity/fragment.
Other than that 👍 👍

As always thank very very much for your valuable contributions!

@AndyScherzinger
Copy link
Member

@tobiasKaminsky I am with @ezaquarii's actual file names, se for example:

So files should start with the "type" So I think, this should be merged as-is.

@tobiasKaminsky
Copy link
Member

I was just looking at our current naming scheme, e.g:
activity_list_item.xml
contactlist_fragment.xml
file_details_fragment.xml

I am totally up for changing this. I just do not want that we have different naming schemes…
I guess we could also have subfolders in layout to split up activity, fragment, etc.

@AndyScherzinger
Copy link
Member

I am totally up for changing this. I just do not want that we have different naming schemes…

absolutely, yes

I guess we could also have subfolders in layout to split up activity, fragment, etc.

I don't think Android is support sub-folders with the layout structure!

@AndyScherzinger
Copy link
Member

@tobiasKaminsky opened #4278 for discussion of naming pattern(s) - I'd still vote to simply merge it (or rename and merge, but not further postpone the merge)

@tobiasKaminsky
Copy link
Member

As we will go with new naming scheme, we can merge this.

@tobiasKaminsky tobiasKaminsky merged commit bc0b9e7 into master Jul 29, 2019
@tobiasKaminsky tobiasKaminsky deleted the ezaquarii/engineering-test-mode branch July 29, 2019 12:52
@AndyScherzinger AndyScherzinger added this to the Nextcloud App 3.8.0 milestone Jul 29, 2019
tobiasKaminsky added a commit that referenced this pull request Jul 30, 2019
129e9ab Merge pull request #4276 from nextcloud/uploader
bc0b9e7 Engineering Test Mode (#4133)
a6371ee check status on transfer update
0938d81 mHostUrlInput can be null if used with direct login (#4270)
53a4844 Merge pull request #4269 from nextcloud/daggerBump
4632dc6 [tx-robot] updated from transifex
2124364 daily dev 20190727
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants