Closed Bug 1254672 Opened 4 years ago Closed 4 years ago

Don't reference fennec_ids.txt in autophone

Categories

(Testing :: Autophone, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: jmaher)

References

Details

Attachments

(1 file)

we shouldn't have a need for fennec_ids.txt and package-name.txt to run tests on android anymore.  

This should be easy in autophone, here is some grep-fu:
jmaher@jmaher-ThinkPad-X230:~/mozilla/autophone$ grep -R fennec_ids * | grep -v autophone.log
builds.py:            # XXX: assumes fixed buildurl-> fennec_ids.txt mapping
builds.py:            fennec_ids_url = urlparse.urljoin(buildurl, 'fennec_ids.txt')
builds.py:            fennec_ids_path = os.path.join(cache_build_dir, 'fennec_ids.txt')
builds.py:            if force or not os.path.exists(fennec_ids_path):
builds.py:                    urllib.urlretrieve(fennec_ids_url, tmpf.name)
builds.py:                    err = 'IO Error retrieving fennec_ids.txt: %s.' % \
builds.py:                        fennec_ids_url
builds.py:                shutil.move(tmpf.name, fennec_ids_path)
tests/runtestsremote.py:                    '--robocop-ids=%s/fennec_ids.txt' % self.parms['build_dir'],


and for package-name.txt:
jmaher@jmaher-ThinkPad-X230:~/mozilla/autophone$ grep -R package-name.txt * | grep -v autophone.log | grep -v build.apk
builds.py:            apkfile.extract('package-name.txt', tmpdir)
builds.py:        with open(os.path.join(tmpdir, 'package-name.txt')) as package_file:
hey :gbrown, can you comment on this if there are other files or things to consider?  I would like to move forward on this bug this week.
Flags: needinfo?(gbrown)
Just some color for package-name.txt -- there are ways to get the Android package name from an APK; for example, using |aapt dump badging|.  If that's not worth it, I'd like to move package-name.txt into assets/, just to stop packing things into the APK root.
No particular complications are coming to mind. I expect fennec_ids.txt is easy to remove now. I haven't thought much about package-name.txt...not as sure about that.
Flags: needinfo?(gbrown)
Do we know when fennec_ids.txt stopped being used? I want to be able to still load it for old builds or branches if necessary without having to check for it.
Flags: needinfo?(jmaher)
(In reply to Bob Clary [:bc:] from comment #4)
> Do we know when fennec_ids.txt stopped being used? I want to be able to
> still load it for old builds or branches if necessary without having to
> check for it.

Bug 969925 made it an empty file.  It wasn't used long before that: Bug 969922.
Flags: needinfo?(jmaher)
fennec_ids.txt stopped being useful in June 2015, long enough that we don't run tests on code that depend on it.

for package-names.txt, we need the process name:
        with open(os.path.join(tmpdir, 'package-name.txt')) as package_file:
            procname = package_file.read().strip()


my understanding is this can change by build type, or branch- is there another way to get this?  it sounds like maybe moving the filename to assets/ would be the best route.  We don't have all the build and android tools on every autophone environment.
:nalexander, if we move package-name.txt to assets/, would we need to modify our code to get the path for package-name.txt:
            build_path = os.path.join(build_dir, 'build.apk')
            apkfile = zipfile.ZipFile(build_path)
            apkfile.extract('application.ini', tmpdir)
            apkfile.extract('package-name.txt', tmpdir)
Flags: needinfo?(nalexander)
Assignee: nobody → jmaher
Status: NEW → ASSIGNED
Attachment #8729062 - Flags: review?(bob)
(In reply to Joel Maher (:jmaher) from comment #7)
> :nalexander, if we move package-name.txt to assets/, would we need to modify
> our code to get the path for package-name.txt:
>             build_path = os.path.join(build_dir, 'build.apk')
>             apkfile = zipfile.ZipFile(build_path)
>             apkfile.extract('application.ini', tmpdir)
>             apkfile.extract('package-name.txt', tmpdir)

Would anybody mind searching in assets/ first, and then falling back to /?  That would let us do these two things independently, at the risk of complicating the test parsing code.
Flags: needinfo?(nalexander)
it seems reasonable to look in assets/ then in /.

In the above code, would we:
try:
  apkfile.extract('assets/package-name.txt', tmpdir)
except:
  apkfile.extract('package-name.txt', tmpdir)

we can do this- just trying to understand where package-name.txt would end up living.
Attachment #8729062 - Flags: review?(bob) → review+
The aapt is part of the build tools which we don't normally install but could, but I wonder if we can't just put something like 

AppName=org.mozilla.fennec in application.ini ?
(In reply to Bob Clary [:bc:] from comment #11)
> The aapt is part of the build tools which we don't normally install but
> could, but I wonder if we can't just put something like 
> 
> AppName=org.mozilla.fennec in application.ini ?

I'm fine with this, if it's doable.
tested the fennec_ids.txt more and merged:
https://github.com/mozilla/autophone/commit/821b575bf0dd2036af821f1b262d227d33c636c5

I am not clear on the next steps for the package-name.txt
(In reply to Joel Maher (:jmaher) from comment #13)
> tested the fennec_ids.txt more and merged:
> https://github.com/mozilla/autophone/commit/
> 821b575bf0dd2036af821f1b262d227d33c636c5
> 
> I am not clear on the next steps for the package-name.txt

Let's leave this for now, then.
(In reply to Joel Maher (:jmaher) from comment #13)

> https://github.com/mozilla/autophone/commit/
> 821b575bf0dd2036af821f1b262d227d33c636c5

deployed 2016-03-15 06:29
shall we close this, or leave it open for package-name.txt changes?
we can close it and open a new bug for package-name.txt or leave as is. Either is fine with me.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Summary: remove references to fennec_ids.txt, package-name.txt from autophone so we can cleanup old build files → Don't reference fennec_ids.txt in autophone
You need to log in before you can comment on or make changes to this bug.