Need a -git make target

Bug #1850156 reported by Jason Boyer
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
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)
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
Revision history for this message
Jane Sandberg (sandbergja) 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.

Revision history for this message
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
Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
Jane Sandberg (sandbergja) wrote :

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

Revision history for this message
Jason Boyer (jboyer) wrote :

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

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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
Changed in evergreen:
milestone: 3.5-beta → 3.5.0
Changed in evergreen:
milestone: 3.5.0 → 3.5.1
Changed in evergreen:
milestone: 3.5.1 → 3.6-beta
Changed in evergreen:
milestone: 3.6-beta → 3.next
Revision history for this message
Jason Stephenson (jstephenson) wrote :

After seeing bug 1940124, I rebased my branch on master to make sure it is up to date. I force pushed over the existing branch.

summary: - Need a -tester make target
+ Need a -git make target
Revision history for this message
Jason Boyer (jboyer) wrote :

Rather than adding new targets I'd now like to propose that firefox-esr and chromium just be dropped from *-developer and the install doc updated to say that you can run the tests if you install the browsers manually.

Branch is here:
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jboyer/lp1850156_bye_bye_browsers / working/user/jboyer/lp1850156_bye_bye_browsers

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Jason Boyer's branch has been signed off and incorporated into the branch for bug 1940146. It seems easiest to build that branch on this one.

tags: added: signedoff
Revision history for this message
Chris Sharp (chrissharp123) wrote :

Pushed to master along with fixes for bug 1940146. Thanks to all!

Changed in evergreen:
status: Confirmed → Fix Committed
milestone: 3.next → 3.8-beta
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.