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

Mock Network revisited... #227

Closed
julianharty opened this issue Aug 9, 2017 · 8 comments
Closed

Mock Network revisited... #227

julianharty opened this issue Aug 9, 2017 · 8 comments
Assignees

Comments

@julianharty
Copy link
Contributor

@mhutti1 In #103 you added support for a mock web-server, and support for 3 specific downloads:

  1. library.xml
  2. test.zim.meta4
  3. testzim.zim

These seem enough to test the download e.g. a list of (1) ZIM file, and enable it to be downloaded.

However, when I try running these tests with the Android device in flight-mode or with WiFi disabled the networkTest fails (as does the downloadTest). Generally when the device has an internet connection the test passes. I wondered whether you intend the tests to be able to run successfully on a device with no active network connection (e.g. when it's in Flight Mode)? If so, perhaps there's a bit more work to do on the code?

I'm happy to help once I understand what's intended to happen.

@mhutti1
Copy link
Contributor

mhutti1 commented Aug 9, 2017

Yeah thats a good point. I guess we should probably look at overriding the network connectivity checks in the test so that they still attempt to download. As well as this on testdroid lots of tests fail due to random device connectivity issues so we might look at fully mocking the network. What are your thoughts on this?

@julianharty
Copy link
Contributor Author

@mhutti1 I'm up for trying to create tests where the network is fully mocked. In my view, we can and should have an option to run tests that test over a real network connection too.

FYI I was toying with several options e.g. running a mini-server using DNS, and/or changing the currently hardcoded
KIWIX_DOWNLOAD_URL = "http://download.kiwix.org/"; (in NetworkModule.java).
It'd be great to have more sophisticated tests where we can inject network delays and errors.

There's lots of options e.g. https://engineering.linkedin.com/blog/2017/03/flashback-mocking-tool

Anyway, and in summary - I'm keen to help improve the testing of network based activities.

@mhutti1
Copy link
Contributor

mhutti1 commented Aug 9, 2017

@julianharty Yeah, thouse sound like good ideas.

@julianharty
Copy link
Contributor Author

Some implementation notes: I've managed to implement a local download server using Apache on a Raspberry Pi running Raspbian. This needs a minimum of 3 files (listed in the order they'd be downloaded by the app):

  1. library_zim.xml
  2. wikipedia_fr_test_2017-07.zim.meta4
  3. wikipedia_fr_test_2017-07.zim

There's a folder structure, the library is expected in ./library. I'd temporarily put the other two files in the root. Again for consistency and reuse they could be in the same structure as used on the main download sites (and mirrors).

In doing so, I realised that URLs in the library and meta4 files need to be updated so they point to the local download server. For now, I hand-crafted these. We could rewrite them on the download server or in the automated tests. I'd prefer to do so on the server as that means the server can be used as-is for other purposes e.g. for hands-on testing of the app locally. Also, having code in the tests to rewrite links is messy and confusing.

Today I hard-code the IP address of the local server at build time by hardcoding the address in the app's build.gradle file. Obviously, this isn't ideal, and also prone to break. I'd like to find a more elegant way to set this. Ideas include:

  • Using an environment variable so that - at least - we don't need to modify the build files to specify a new address.
  • Using a name rather than an IP address. Here the challenge is in either editing the hosts file on the Android device (which requires root access as far as I can tell so far in my experiments) or getting the name added in a local DNS on the network.
  • Using a 'known' address e.g. x.x.x.101 on the same subnet (where the rest of the address is calculated using the device's assigned IP address).
  • Using a discovery mechanism.

We could also consider adding a way to set the address/name at run time, in the settings for the app. I think we're currently cautious - as a team - to do so, as this may confuse users and cause problems if it's set correctly for end users. However, perhaps we could make this available based on the build variant of the app and only expose it in debug-based build variants?

PS: I'm thinking in IPv4 terms. Perhaps IPv6 has additional features/options that help?

@julianharty
Copy link
Contributor Author

Another note: We could edit the library_zim.xml to include some zim files with known characteristics e.g. corrupt and missing entries and keep these away from the production site. Having these additional files can make it easier to test how the app behaves with flawed ZIM files.

@kelson42 kelson42 added this to the 2.5 milestone Apr 19, 2019
@macgills macgills modified the milestones: 2.5, 3.0 May 27, 2019
@kelson42 kelson42 removed this from the 3.0 milestone Jun 4, 2019
@kelson42
Copy link
Collaborator

@julianharty @mhutti1 @macgills library_zim.xml will disappear soon. I propose to close this ticket. The ticket does also not explain why tests can not run against the production infrastructure (which it should IMO).

@macgills
Copy link
Contributor

I am okay with closing this. I am plotting a full revamp of the automation tests as is

@julianharty
Copy link
Contributor Author

@macgills OK, I agree with fully revamping the automation tests. Some of the main causes of crashes in production seem to be either device, content, or lifecycle related. Be great to have as many as practical of these covered in the revamped suite.

I'll close this ticket.

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

No branches or pull requests

4 participants