Need a -tester make target

Bug #1850156 reported by Jason Boyer on 2019-10-28
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Wishlist
Unassigned

Bug Description

This is a followup to bug 1845693 (replacing PhantomJS). In order to properly test the angular clients now a full install of Chromium and Firefox are installed whenever the <os-suite>-developer pre-requisite target is run. Knowing that this may be overkill for many we should instead add a target for <os-suite>-tester instead. Additionally this could pull in pgtap package and any perl packages that are only useful for or simplify testing. It could depend on the -developer package to make it a simpler target.

Jason Boyer (jboyer) on 2019-10-28
Changed in evergreen:
importance: Undecided → Wishlist
milestone: none → 3.4.2
milestone: 3.4.2 → 3.next
milestone: 3.next → none
Changed in evergreen:
milestone: none → 3.5-alpha
no longer affects: evergreen/3.5
Jane Sandberg (sandbej) wrote :

I want to gently push back on separating the out the -tester pre-requisites. I feel that we should be making it as easy as possible for developers to run tests, and less easy to opt out of running tests. I do understand the concern about overkill, though, so take this with a grain of salt.

I am fully in support of adding pgtap to some target; that would certainly make it easier for developers to run tests.

Jason Boyer (jboyer) wrote :

In writing my reply as to why this was so important, I realized that I misremembered the instructions recommending that you use -developer any time when installing from git. Since that is *not* the case, so thanks for the gentle nudge Jane. :) Unless someone comes along with a better (and accurate) argument as to why splitting it out is necessary, I'm going to mark this as invalid.

Changed in evergreen:
status: New → Invalid
no longer affects: evergreen/3.2
no longer affects: evergreen/3.3
no longer affects: evergreen/3.4
Changed in evergreen:
status: Invalid → Confirmed
Jason Stephenson (jstephenson) wrote :

I'm setting this bug to confirmed because the *-developer Makefile.install target is required to install from a git checkout. Try installing on a fresh VM without installing that target. You won't get very far.

While I agree with Jane's sentiment about wanting developers to run tests, I install from git exclusively for production and I don't need web browsers, etc., installed on these machines.

My suggestion is that instead of splitting the testing items into a -testing target, that we could add the things necessary to install from git into a *-git target and modify *-developer to also run the *-git Makefile.install target.

Jason Stephenson (jstephenson) wrote :

After updating this bug, I took a look at making a branch and discovered that the only thing truly needed is the install_nodejs_from_source target in the Open-ILS/src/extras/install/Makefile.common.

The other necessary packages, autoconf, automake, and libtool, should be install as part of the prerequisites when installing OpenSRF.

So, another option could be to document the need for those packages and the nodejs installation target in the README for those installing from git.

Whatever the consensus is, I'll be happy to do a branch.

Jason Stephenson (jstephenson) wrote :

OK, so forget my previous comment because trying to run install_nodejs_from_source by itself results in an error:

sudo make -f Evergreen/Open-ILS/src/extras/install/Makefile.common install_nodejs_from_source
wget -N
wget: missing URL
Usage: wget [OPTION]... [URL]...

Try `wget --help' for more options.
Evergreen/Open-ILS/src/extras/install/Makefile.common:38: recipe for target 'install_nodejs_from_source' failed
make: *** [install_nodejs_from_source] Error 1

We're going to need Makefile changes to accommodate this.

Jane Sandberg (sandbej) wrote :

+1 to a *-git target and a *-developer target as you described, Jason S.

Jason Boyer (jboyer) wrote :

Agreed, *-git sounds *-good to me.

Jason Stephenson (jstephenson) wrote :

Now that I've actually coded this up, I think a -testing target is better.

By making the developer target require the git target, you can end up installing Node twice if you first run the git target and then run the developer target.

The developer target now exists for only 2 packages that are required for testing. Everything else from the developer target is required to build via git.

By leaving developer alone and moving chromium and firefox to a separate testing target, it makes much more sense to add pgtap to the new, testing target.

We can also not make the testing target depend on the developer target, but document that the testing target recommends you install the developer target first.

Also, the testing target can be installed and the tests run by someone who has installed from a tarball without requiring the installation of the developer target, I think. (This last one needs to be tried.)

So, now the original intent of this bug makes much more sense to me.

Jason Stephenson (jstephenson) wrote :

And... After staring to implement the -tester variation and discovering that there is no easy way to install pgtap from the Makefile.install because which package you should install depends very much on which version of PostgreSQL is installed, I realized that the main benefit of having a tester install target is gone.

So, I'm back to implementing a git target, but with a difference in how I started out doing it.

Jason Stephenson (jstephenson) wrote :

Here's a branch that add the <osname>-git Makefile.install target, updates the existing targets to use it as appropriate, and updates the README with documentation on the new target:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1850156-git-makefile.install-target

tags: added: pullrequest
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers