Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Add UI tests nightly build #17948

Merged

Conversation

isabelrios
Copy link
Contributor

Creating a new PR to continue the work and investigate the issue found in the decission task previously when trying to add a test for Nightly builds, similar to the one added recently to Beta builds.

As commented in that PR: The idea is to run at least one test on the Nightly build in addition to the tests that run in the debug build so that if it fails we know something is not correct with the Nightly build that is used by the cron job to uplift it to the playstore. Currently there are only unit tests checking the Nightly build that is created by the cron job.

@isabelrios
Copy link
Contributor Author

@bhearsum please let me ask you for review in this PR... This is a new PR to try to solve the issue we commented a few days ago on slack about adding a new task to run UI tests on Nightly builds.

Here in this PR looks like the ui-test-x86-nightly task is created successfully. I added the try_config file to check that, is there any other way I can try to know this is going to be fine on master and not break the decission task?

@@ -49,13 +49,14 @@ job-template:
# No test job runs on push against this build type. Although we do want nightly-simulation
# signed to use it in the gecko trees.
# We want beta on push so that we detect problem before shipping it
(nightly-simulation|beta-firebase|android-test-beta): [github-push]
(nightly-simulation|beta-firebase|android-test-beta|nightly-firebase|android-test-nightly): [github-push]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bhearsum I forgot to add this in previous PR that failed, do you think this would make any difference to avoid the issue I had before?

This was the previous PR and the issue

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this may fix the issue; it's worth a try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!! Let's land it and try...if it fails I will revert it right away

@isabelrios
Copy link
Contributor Author

@bhearsum I remember you mentioned about changing the build type to something different than nightly. I have tried that but the only option I can think of would be to create a new build type in build.gradle and there are geckoview compilation issues.

Not sure if there are other options to modify that...

@isabelrios isabelrios marked this pull request as ready for review February 11, 2021 08:49
@isabelrios isabelrios requested review from a team as code owners February 11, 2021 08:49
@isabelrios isabelrios merged commit 0328799 into mozilla-mobile:master Feb 11, 2021
isabelrios added a commit to isabelrios/fenix that referenced this pull request Feb 11, 2021
isabelrios added a commit that referenced this pull request Feb 11, 2021
@isabelrios
Copy link
Contributor Author

@bhearsum Unfortunatelly same error as before:

[task 2021-02-11T09:02:06.069Z] 2021-02-11 09:02:06,069 - INFO - Creating task with taskId FQPA_EOaS3WJalBdXp2d-Q for signing-android-test-nightly
.....
 * method:     createTask
[task 2021-02-11T09:02:06.104Z] * errorCode:  InsufficientScopes
[task 2021-02-11T09:02:06.104Z] * statusCode: 403
[task 2021-02-11T09:02:06.104Z] * time:       2021-02-11T09:02:06.489Z
.........
[task 2021-02-11T09:02:06.111Z] Traceback (most recent call last):
[task 2021-02-11T09:02:06.111Z]   File "/builds/worker/.local/lib/python2.7/site-packages/taskgraph/main.py", line 444, in ....
[task 2021-02-11T09:02:06.114Z] HTTPError: 403 Client Error: Forbidden for url: http://taskcluster/queue/v1/task/FQPA_EOaS3WJalBdXp2d-Q

Something still wrong trying to sign the android-test-nightly....
I know you are really busy but it would be nice if we can set up sometime to try to figure this out.
Thanks!

@escapewindow
Copy link
Contributor

Do we need this signed?
We don't want to grant production signing scopes to github-push, but we're creating a production signing task.
If we're ok with it, we could do dep signing or similar, or skip signing.

@isabelrios
Copy link
Contributor Author

Do we need this signed?
We don't want to grant production signing scopes to github-push, but we're creating a production signing task.
If we're ok with it, we could do dep signing or similar, or skip signing.

@aki thanks for jumping in... We need the apk to be signed but it does not have to be with the production signing task... that's not intended in this PR and not needed so if we can do dep signing that would be great...

@escapewindow
Copy link
Contributor

So in this block we want dep-signing, but because we're nightly and level 3, we get signing.

In this block we want dep-signing, but because we're nightly and level 3, we get production-signing.

Can we make the build not have the nightly build type? That would solve this issue easily. If that defeats the purpose of adding the test, we may need to make those blocks even more complex, and/or add transforms to force dep-signing for these test builds.

@isabelrios
Copy link
Contributor Author

Oh I see.. I'm afraid I don't know how to use a different build type in order to get the nightly build for testing... these are the build types we have: https://github.com/mozilla-mobile/fenix/blob/master/app/build.gradle#L92 . I have followed same implementation as for the debug and beta tests that we have... These were the changes to allow the beta tests work in case that helps.

I need the nightly build type so that it is used here and the correct apks for nighly are generated.

If there is already a nightly being created per push in master and that's signed, not sure if we could use that one? Then we should only need to create the test apk... in theory the testApk and apk could be signed with different signature as long as they are signed, tests will run in Firebase.
Not sure if that's an option...thinking out loud...
Thanks for your help @escapewindow

@escapewindow
Copy link
Contributor

@isabelrios what is the test? Is the test this nightly build task, or are we running a UI test against a nightly build?
If the latter, can we create a test task that uses the previous nightly build's index or maven artifacts?

@isabelrios
Copy link
Contributor Author

isabelrios commented Feb 16, 2021

@escapewindow the test we want to run is a UI test defined here.
The idea is to run this UI test against a nightly build. With this PR we get the correct apks for that which are: org.mozilla.fenix.nightly and org.mozilla.fenix.nightly.test If the nightly build is already created in a previous/existing task, it would be enough to get the test apk as long as it is signed (no need prod signature or the same as the nightly apk, just a signature) . But we need both to be uploaded to Firebase so that the test runs.

@escapewindow
Copy link
Contributor

@isabelrios Hm.

  • Do we need the nightly build to come from the same revision of the merge, or do we just need to test the latest nightly? If we can do the latter, then we may be able to find the latest nightly, upload it to Firebase, and run the test.
  • Is there something about the nightly build that's different from a non-nightly build? If we need a build run on push that we test in Firebase, I'm hoping we can use a non-nightly build.

I'm getting the impression that we need a new nightly build with dep-signing on push, in which case we might need a patch like this.

@isabelrios
Copy link
Contributor Author

Hope I can answer your questions @escapewindow ...

  • It would be better if we can build a nightly build from each commit on master so that we run the UI tests against it and in case something is wrong, we will know before the nightly build is created from the cron job. That was the initial idea to have this test (future tests) in place.

  • Yes, the apks and so the build is different. (let me share some links to the different builds running on Firebase: beta, nightly (while doing my tests to the PR) and debug)

Some context here is that currently on master we are only running tests against the debug build and we are missing that coverage for nightly builds. On releases branches we run tests against beta builds. Each type of build should be covered somehow. At the moment the nightly build created and uploaded is only checked by the unit tests.

Hope this makes sense...

I'm getting the impression that we need a new nightly build with dep-signing on push, in which case we might need a patch like this.

Yes, that sounds like what we want but worried if that patch will complicate things considerably...

@escapewindow
Copy link
Contributor

I'm getting the impression that we need a new nightly build with dep-signing on push, in which case we might need a patch like this.

Yes, that sounds like what we want but worried if that patch will complicate things considerably...

Yes, that's why in #17948 (comment) I said

Can we make the build not have the nightly build type? That would solve this issue easily. If that defeats the purpose of adding the test, we may need to make those blocks even more complex, and/or add transforms to force dep-signing for these test builds.

We've probably passed the point where encoding the config in yaml + keyed_by is more useful than obfuscating. We may want to take out worker-type, signing-type, and index logic from the kind.yml completely, and add them back in in the taskcluster/fenix_taskgraph/transforms/signing.py transforms. Something like this.

@isabelrios
Copy link
Contributor Author

We've probably passed the point where encoding the config in yaml + keyed_by is more useful than obfuscating. We may want to take out worker-type, signing-type, and index logic from the kind.yml completely, and add them back in in the taskcluster/fenix_taskgraph/transforms/signing.py transforms. Something like this.

Thanks @escapewindow for all your explanation, to be honest I don't have enough knowledge about TC or tashkgraph to know/understand if that change will have more pros or cons for now and in the future :( Don't know the consequences in terms of complexity for further implementations...
Also kind of worried that we can not test whether this is going to work or no until we have it in "production" and face similar issue as with these PRs I tried to land, but if you feel confident this could be the solution I'm happy to try that out and help as much as I can to have this working

Thanks again for all your help with this

@escapewindow
Copy link
Contributor

We've probably passed the point where encoding the config in yaml + keyed_by is more useful than obfuscating. We may want to take out worker-type, signing-type, and index logic from the kind.yml completely, and add them back in in the taskcluster/fenix_taskgraph/transforms/signing.py transforms. Something like this.

Thanks @escapewindow for all your explanation, to be honest I don't have enough knowledge about TC or tashkgraph to know/understand if that change will have more pros or cons for now and in the future :( Don't know the consequences in terms of complexity for further implementations...

I think the above patch should be ok.

Also kind of worried that we can not test whether this is going to work or no until we have it in "production" and face similar issue as with these PRs I tried to land, but if you feel confident this could be the solution I'm happy to try that out and help as much as I can to have this working

https://docs.mozilla-releng.net/en/latest/procedures/mobile_automation_setup.html?highlight=mobile#how-to-set-up-taskgraph-for-mobile describes how we can test taskgraph changes without testing in production. The last step (taskgraph-gen.py) allows us to generate a set of task graphs before we make a change, and compare it to what it looks like after we make a change. I did this, and the changes look like they're probably good. If you're going to be adding tasks and changing task configurations frequently, it's probably worth trying out.

Thanks again for all your help with this

No problem!

@isabelrios
Copy link
Contributor Author

okay, sounds good...should I go ahead and create a PR with those changes then @escapewindow? not sure if you prefer to do the changes in the taskgraph/signing kind.yml and then I re-do my PR or I can create the PR with that? As you prefer... :)

@escapewindow
Copy link
Contributor

If you're ok with it, please feel free to include those changes in a PR.

@isabelrios
Copy link
Contributor Author

If you're ok with it, please feel free to include those changes in a PR.

@escapewindow Okay, I will create a new PR with those changes plus the original ones in this PR trying to have test running on Nightly. Thanks!

pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this pull request Aug 4, 2021
* Try adding UI Tests to Nightly build

* adding try_config file

* remove try_task_config
pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this pull request Aug 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants