1017484 - Use appropriate dependencies for the branch under test

Status: RESOLVED FIXED

(Testing Graveyard :: Eideticker, defect)

Description

[Dave Hunt [:davehunt] [he/him] ⌚BST]

At the moment Eideticker is using gaiatest and b2gpopulate packages that are only guaranteed to support the master and mozilla-central branches. In bug 994176 we're going to be producing versions of these packages for the release branches such as v1.3 and v1.4.

Ideally we would avoid branching Eideticker in order to support these branches. Perhaps we could move these requirements out to files such as requirements-mozilla-b2g28_v1_3.txt and select the appropriate file based on the build under test.

This would mean we could only use methods used from these packages that are common across the branches, otherwise we'd need additional logic in Eideticker or would need to branch.

Comment 1

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Will: Any thoughts on this? I can experiment with splitting out the requirements into text files and conditionally picking one based on the target build.

Comment 2

[William Lachance (:wlach)]

(In reply to Dave Hunt (:davehunt) from comment #1)
> Will: Any thoughts on this? I can experiment with splitting out the
> requirements into text files and conditionally picking one based on the
> target build.

Yup, that makes sense to me. Note that you might need to modify setup.py in src/eideticker/setup.py to no longer explicitly list the dependencies, since the exact packages we're using will depend on the version.

Comment 3

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I was thinking more about this and I don't think we can avoid branching. There are a couple of changes in the pipeline for gaiatest that will change our imports, so just depending on the appropriate version won't be enough. Maybe we could split the B2G parts into a submodule and only branch that, but I fear that would cause an even bigger headache.

Comment 4

[William Lachance (:wlach)]

Could we create a compatibility wrapper/module that we could import selectively for backwards compatibility? Ugly, but less so than branching most likely.

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Do you mean abstract gaiatest from Eideticker, and in this abstraction handle the branch appropriate imports and method calls? I think that would work, and is certainly less ugly than branching. We're only using gaiatest in a few places, and means all the ugliness and backward compatibility logic will be in one place.

Comment 6

[Mike Lee [:mlee]]

Dave,
Are you taking this bug? It's blocking your existing patch for bug 1002502 but has no assignee.

Comment 7

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Mike Lee [:mlee] from comment #6)
> Dave,
> Are you taking this bug? It's blocking your existing patch for bug 1002502
> but has no assignee.

Most likely, but this bug is also blocked on a bug that's assigned to me. :)

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I have an early proof-of-concept ready for feedback. Unfortunately this is sitting on my Eideticker dev laptop at home and I'm in the London office today. I'll try to remember to upload it this evening when I get home.

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Use appropriate dependencies for the branch under test. v1.0

Comment 10

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Use appropriate dependencies for the branch under test. v1.0

This is the correct patch.

Comment 11

[William Lachance (:wlach)]

Comment on attachment 8438336 [details] [diff] [review]
Use appropriate dependencies for the branch under test. v1.0

This all looks pretty reasonable to me!

Comment 12

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8438336 [details] [diff] [review]
Use appropriate dependencies for the branch under test. v1.0

Okay, I've tried (and failed) to replicate the update notifications on Tarako, so I think this is safe to request review. If the update notifications do show up, we can address this in bug 1023284 with a follow-up to this patch.

Comment 13

[William Lachance (:wlach)]

Comment on attachment 8438336 [details] [diff] [review]
Use appropriate dependencies for the branch under test. v1.0

lgtm, as before

Comment 14

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Use appropriate dependencies in CI. v1.0

This patch is for Eideticker CI, as the two should be landed at the same time.

Comment 15

[William Lachance (:wlach)]

Comment on attachment 8442009 [details] [diff] [review]
Use appropriate dependencies in CI. v1.0

Review of attachment 8442009 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome!

Comment 16

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Landed in:
https://github.com/mozilla/eideticker/commit/8820e3008cdc4e395c62865972a9202144940eb5
https://github.com/mozilla/eideticker-ci/commit/0a91c8ad2ade80e9c256bdb2097b586acef4bde8

Updated Eideticker CI and triggered a Tarako build to test it:
http://eideticker-ci1.ateam.phx1.mozilla.com:8080/job/b2g.tarako.mozilla-b2g28_v1_3t.tests/65/console

I'm going to hold off marking this fixed until I see a successful job.