Skip to content

Conversation

@klown
Copy link

@klown klown commented Jul 16, 2014

No description provided.

klown added 4 commits July 16, 2014 09:42
Reworked the tests to check whether tuxguitar is installed.  Based
on that check, did not run tests that depend on it being installed,
and printed a message on the console advertising that fact.

Added back the updatePackage() test since that check was already
being made.
Checked that tuxguitar is in fact installed prior to running the
'remove' test.  If not installed, prints a message on the
console to that effect, and does not run the 'remove' test.
Used 'glib2' for searchPackage() test and '/usr/bin/ls' for
searchFiles() test since they exist and are installed.

Modified searchPackage() "installed" vs. "~installed" tests
basing them on whether tuxguitar is installed or not.

Rearranged and general cleanup.
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like this if-statement is not coherent, this is,
if (isInstalled (tuxguitarPkg)) then jqUnit.assertTrue ("Did not find tuxguitar in installed packages", installed)
shouldn't be
if (isInstalled (tuxguitarPkg)) then jqUnit.assertTrue ("Found tuxguitar in installed packages", installed) ???

Copy link
Author

Choose a reason for hiding this comment

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

My understanding is that the message is printed when the test fails -- is that incorrect?

In this specific case, the expected result is that the package is in the list of installed packages. If that fails, then the reason for the failure is printed: "Did not find tuxguitar in installed packages".

Would this be clearer: "Expected to find tuxguitar in list of installed packages, but didnt'"?

@javihernandez
Copy link
Owner

In addition to my comments, by having tuxguitar installed on my system, the tests fail when running them.
Looks like tuxguitar is still found in "~installed" when installed, at least in my system, and that makes the assert on line 103 (https://github.com/klown/linux/blob/device-reporter-draft2/node_modules/packagekit/nodepackagekit/nodepackagekit_test.js#L103) fail.

Also, shouldn't we perform the install and remove tasks in both cases? I mean, what if:

  • Check if tuxguitar is installed on the system
    ** yes: perform remove and then re-install it again
    ** no: perform install and then remove it

This won't affect the user because his personal config of tuxguitrar is stored into his home folder (which won't be removed during the uninstall task). So we would be performing all the tests In both cases (by having and by not having tuxguitar installed on the system)

@klown
Copy link
Author

klown commented Jul 23, 2014

In addition to my comments, by having tuxguitar installed on my system, the tests fail when
running them. Looks like tuxguitar is still found in "~installed" when installed
...
I checked both configurations on my system: either install or remove tuxguitar before running the tests. Everything worked here. Is it possible you installed tuxguitar without using any package manager? I don't see any way around that, unless ...

Also, shouldn't we perform the install and remove tasks in both cases? I mean, what if:

Check if tuxguitar is installed on the system ** yes: perform remove and then re-install it again ** > no: perform install and then remove it

I'll try that.

@javihernandez
Copy link
Owner

@klown, look

jhernandez@jhernandez-laptop:~/git/github/GPII_javihernandez_linux/node_modules/packagekit/nodepackagekit> rpm -qa|grep tuxguit
tuxguitar-1.2-12.1.1.i586
nojhernandez@jhernandez-laptop:~/git/github/GPII_javihernandez_linux/node_modules/packagekit/nodepackagekit> node
> var packagekit = require("./build/Release/nodepackagekit.node");
undefined
> packagekit.searchPackage("tuxguitar", "~installed")
[ { id: 'tuxguitar;1.2-12.1.1;i586;repo-oss',
    name: 'tuxguitar',
    version: '1.2-12.1.1',
    data: 'repo-oss' },
  { id: 'tuxguitar;1.2-12.1.1;x86_64;repo-oss',
    name: 'tuxguitar',
    version: '1.2-12.1.1',
    data: 'repo-oss' } ]
> packagekit.searchPackage("tuxguitar", "installed")
[ { id: 'tuxguitar;1.2-12.1.1;i586;installed',
    name: 'tuxguitar',
    version: '1.2-12.1.1',
    data: 'installed' } ]

@klown
Copy link
Author

klown commented Jul 23, 2014

@javihernandez, I'm trying to figure out what that means. It looks like there are two i586 based packages, with identical version numbers, one of which is installed, and one isn't. That makes no sense. Is it because they come from different repositories? I'm baffled.

Out of curiosity, what happens if you do the following?
$ pkcon search name tuxguitar

My guess is something like:
Installed tuxguitar;1.2-12.1.1;i586
Available tuxguitar;1.2-12.1.1;i586 (repo-oss)
Available tuxguitar;1.2-12.1.1;x86_64 (repo-oss)

My puzzlement is: if the unit test removes the first one, which one comes up when the test tries to re-install it? Note that the full package id will change.

@javihernandez
Copy link
Owner

pkcon search name tuxguitar returns:

Installed       tuxguitar-1.2-12.1.1.x86_64                 A multitrack tablature editor and player written in Java-SWT
Available       tuxguitar-1.2-12.1.1.i586                   A multitrack tablature editor and player written in Java-SWT

About your puzzlement, if it's installed, can't we just keep the original version (id) somewhere in a variable and then install that one again? ie:
Given our original (and already installed) tuxguitar's package id 'tuxguitar;1.2-12.1.1;i586;installed', we can search for the package's id we want to install again by looking for the string 'tuxguitar;1.2-12.1.1;i586' among the available packages. Seem reasonable?

@klown
Copy link
Author

klown commented Jul 23, 2014

'm even more baffled than before since pkcon is returning only two packages whereas the packagekit addon returned three. And what pkcon is reporting as installed is completely opposite to what you showed previously using the addon. But, they are running the same code, ultimately. Arggh!

As for using the same id: not exactly. I think if we keep and compare the "name" and "version" fields from our objects we will be okay. But, not the full id, because that includes info about availability, updates, etc. For example, here are the ids on my system depending on whether the package is installed or not. The ids are not identical, but the name and version numbers are.

tuxguitar;1.2-15.fc20;x86_64;fedora
tuxguitar;1.2-15.fc20;x86_64;installed:fedora

I'm not sure about your system since you showed two with identical names, versions, and architectures, but different "data" fields ("installed" vs. "repo-oss").

By the way, our data structure does not show the architecture (i586 vs. x86_64), so it can' t distinguish between packages by that criterion. I'm wondering if we should take that into account.

@javihernandez
Copy link
Owner

Right, probably we should include the "arch" [1] field and use that (name+version+arch) as our criteria.

[1] http://www.freedesktop.org/software/PackageKit/gtk-doc/PkPackage.html#pk-package-get-arch

klown added 2 commits July 23, 2014 15:57
- nodpackagekit.cc: Retrieves architecture info from the package
id and stores it in the new "arch" field of the package JS objecgts.

- nodepakagekit_test.js:  Uses "arch" field to match package
across the install/remove tests.  Also, made the assertX() messages
more generic.
Used new "arch" field to insure that the package that is checked
for in the 'installed' or 'available' lists takes the architecture
information into account.
javihernandez added a commit that referenced this pull request Jul 24, 2014
Latest changes on nodepackagekit_tests.js
@javihernandez javihernandez merged commit 7ad7192 into javihernandez:device-reporter-draft-2 Jul 24, 2014
@javihernandez
Copy link
Owner

Thanks, now it's perfect! 🍻

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

Successfully merging this pull request may close these issues.

2 participants