#7 Implement test mode
Merged 3 years ago by salimma. Opened 3 years ago by salimma.
salimma/fedora-business-cards add-test-mode  into  master

Test file generation
Michel Alexandre Salim • 3 years ago  
Move FAS lookup to a common method
Michel Alexandre Salim • 3 years ago  
Add `--test` option to generators
Michel Alexandre Salim • 3 years ago  
Make generators share extra_options
Michel Alexandre Salim • 3 years ago  
Fix typo
Michel Alexandre Salim • 3 years ago  
file modified
+5
@@ -24,6 +24,11 @@ 

  Nick Bebout (Fedora card layout fixes)

  Michael Scherer (security fixes)

  Brian Exelbierd (continued maintenance)

+ Michel Alexandre Salim (testing)

+ 

+ --

+ 

+ To test, run `python3 -m unittest discover`

  

  --

  

file modified
+1 -1
@@ -1,5 +1,5 @@ 

  #!/usr/bin/python3

- # This file is provided as a conveinence to people using the Git repository.

+ # This file is provided as a convenience to people using the Git repository.

  # It is not included in source distributions, and it is created in binary

  # distributions as a setuptools entry point.

  from fedora_business_cards.frontend.cmdline import main

@@ -23,6 +23,10 @@ 

  card layout, a Beefy Miracle business card layout).

  """

  

+ from fedora.client.fas2 import AccountSystem

+ from getpass import getpass

+ 

+ from fedora_business_cards import __version__

  from fedora_business_cards import common

  

  
@@ -35,6 +39,10 @@ 

      unit = None

      rgb_to_cmyk = None

  

+     # These should be overridden by subclasses

+     _gen_name = 'base'

+     _gen_desc = 'Base generator'

+ 

      def __init__(self, options):

          self.options = options

          self.height = options.height
@@ -44,9 +52,39 @@ 

              raise KeyError(options.unit)

          self.unit = options.unit

  

-     @staticmethod

-     def extra_options(parser):

-         return None

+     @classmethod

+     def extra_options(cls, parser):

+         option_group = parser.add_parser(cls._gen_name, help=cls._gen_desc)

+         option_group.add_argument('-u', '--username', dest='username',

+                                   default='', help='If set, use a different name'

+                                   ' than the one logged in with to fill out'

+                                   ' business card information')

+         option_group.add_argument('-t', '--test', dest='test',

+                                   action='store_true', help='If set, use test data'

+                                   ' to fill out business card information')

+         return option_group

+ 

+     def collect_fas_information(self):

+         if self.options.test:

+             return {

+                 'human_name': 'Jane Doe',

+                 'gpg_keyid': '0xGPGKEYID',

+                 'ircnick': 'jane_doe',

+                 'username': self.options.username or 'jane_doe',

+             }

+         else:

+             # ask for FAS login

+             print("Login to FAS:")

+             username = input("Username: ")

+             password = getpass()

+ 

+             # get information from FAS

+             fas = AccountSystem(username=username, password=password,

+                                 useragent='fedora-business-cards/%s' % __version__)

+             if self.options.username:

+                 username = self.options.username

+             return fas.person_by_username(username)

+ 

  

      def collect_information(self):

          pass

@@ -31,22 +31,20 @@ 

  import importlib.resources as pkg_resources

  import argparse

  from decimal import Decimal

- from getpass import getpass

  from xml.dom import minidom

  from builtins import input

  

- from fedora_business_cards import __version__

  from fedora_business_cards import common

  from fedora_business_cards.generators import BaseGenerator

  from fedora_business_cards import templates  # relative-import the *package* containing the card templates

  

- AccountSystem = \

-         common.recursive_import('fedora.client.fas2', True).AccountSystem

- 

  FEDORA_LOGO_VIEWBOX = '100 100 707.776 215.080'

  

  

  class FedoraHorizontalGenerator(BaseGenerator):

+     _gen_name = 'fedora-horizontal'

+     _gen_desc = 'Fedora horizontal business cards'

+ 

      rgb_to_cmyk = {

              (60, 110, 180): (1, .46, 0, 0),

              (41, 65, 114): (1, .57, 0, .38),
@@ -54,33 +52,14 @@ 

              (255, 255, 255): (0, 0, 0, 0),

      }

  

-     @staticmethod

-     def extra_options(parser):

-         option_group = parser.add_parser('fedora-horizontal', help='Fedora horizontal business cards')

-         option_group.add_argument('-u', '--username', dest='username',

-                                   default='', help='If set, use a different name'

-                                   ' than the one logged in with to fill out'

-                                   ' business card information')

-         return option_group

- 

      def collect_information(self):

-         # ask for FAS login

-         print("Login to FAS:")

-         username = input("Username: ")

-         password = getpass()

- 

-         # get information from FAS

-         fas = AccountSystem(username=username, password=password,

-                             useragent='fedora-business-cards/%s' % __version__)

-         if self.options.username:

-             username = self.options.username

-         userinfo = fas.person_by_username(username)

+         userinfo = self.collect_fas_information()

  

          # set business card fields

          self.fields['name'] = userinfo["human_name"]

          self.fields['title'] = "Fedora Project Contributor"

          self.fields['lines'] = [''] * 6

-         self.fields['lines'][0] = '%s@fedoraproject.org' % username

+         self.fields['lines'][0] = '%s@fedoraproject.org' % userinfo['username']

          self.fields['lines'][1] = 'fedoraproject.org'

          next_line = 2

          if userinfo['ircnick']:
@@ -99,7 +78,8 @@ 

          def cmdline_card_line(data):

              return "| %s%s |" % (data, ' ' * (59 - len(data)))

  

-         done_editing = False

+         # don't prompt user to edit in test mode

+         done_editing = self.options.test

          while not done_editing:

              print("Current business card layout:")

              print("   +" + "-" * 61 + "+")

@@ -31,22 +31,20 @@ 

  import importlib.resources as pkg_resources

  import argparse

  from decimal import Decimal

- from getpass import getpass

  from xml.dom import minidom

  from builtins import input

  

- from fedora_business_cards import __version__

  from fedora_business_cards import common

  from fedora_business_cards.generators import BaseGenerator

  from fedora_business_cards import templates  # relative-import the *package* containing the card templates

  

- AccountSystem = \

-         common.recursive_import('fedora.client.fas2', True).AccountSystem

- 

  FEDORA_LOGO_VIEWBOX = '100 100 707.776 215.080'

  

  

  class FedoraVerticalGenerator(BaseGenerator):

+     _gen_name = 'fedora-vertical'

+     _gen_desc = 'Fedora vertical business cards'

+ 

      rgb_to_cmyk = {

              (60, 110, 180): (1, .46, 0, 0),

              (41, 65, 114): (1, .57, 0, .38),
@@ -54,33 +52,14 @@ 

              (255, 255, 255): (0, 0, 0, 0),

      }

  

-     @staticmethod

-     def extra_options(parser):

-         option_group = parser.add_parser('fedora-vertical', help='Fedora vertical business cards')

-         option_group.add_argument('-u', '--username', dest='username',

-                                   default='', help='If set, use a different name'

-                                   ' than the one logged in with to fill out'

-                                   ' business card information')

-         return option_group

- 

      def collect_information(self):

-         # ask for FAS login

-         print("Login to FAS:")

-         username = input("Username: ")

-         password = getpass()

- 

-         # get information from FAS

-         fas = AccountSystem(username=username, password=password,

-                             useragent='fedora-business-cards/%s' % __version__)

-         if self.options.username:

-             username = self.options.username

-         userinfo = fas.person_by_username(username)

+         userinfo = self.collect_fas_information()

  

          # set business card fields

          self.fields['name'] = userinfo["human_name"]

          self.fields['title'] = "Fedora Project Contributor"

          self.fields['lines'] = [''] * 5

-         self.fields['lines'][0] = '%s@fedoraproject.org' % username

+         self.fields['lines'][0] = '%s@fedoraproject.org' % userinfo['username']

          self.fields['lines'][1] = 'fedoraproject.org'

          next_line = 2

          if userinfo['ircnick']:
@@ -100,7 +79,8 @@ 

          def cmdline_card_line(data):

              return "| %s%s |" % (data, ' ' * (35 - len(data)))

  

-         done_editing = False

+         # don't prompt user to edit in test mode

+         done_editing = self.options.test

          while not done_editing:

              print("Current business card layout:")

              print("Vertical cards don't hold much data ...:")

@@ -27,21 +27,19 @@ 

  import sys

  import argparse

  from decimal import Decimal

- from getpass import getpass

  from xml.dom import minidom

  from builtins import input

  

- from fedora_business_cards import __version__

  from fedora_business_cards import common

  from fedora_business_cards.generators import BaseGenerator

  

- AccountSystem = \

-         common.recursive_import('fedora.client.fas2', True).AccountSystem

- 

  FEDORA_LOGO_VIEWBOX = '100 100 707.776 215.080'

  

  

  class FedoraGenerator(BaseGenerator):

+     _gen_name = 'fedora'

+     _gen_desc = 'Fedora original horizontal business cards'

+ 

      rgb_to_cmyk = {

              (60, 110, 180): (1, .46, 0, 0),

              (41, 65, 114): (1, .57, 0, .38),
@@ -49,27 +47,8 @@ 

              (255, 255, 255): (0, 0, 0, 0),

      }

  

-     @staticmethod

-     def extra_options(parser):

-         option_group = parser.add_parser('fedora', help='Fedora original horizontal business cards')

-         option_group.add_argument('-u', '--username', dest='username',

-                                   default='', help='If set, use a different name'

-                                   ' than the one logged in with to fill out'

-                                   ' business card information')

-         return option_group

- 

      def collect_information(self):

-         # ask for FAS login

-         print("Login to FAS:")

-         username = input("Username: ")

-         password = getpass()

- 

-         # get information from FAS

-         fas = AccountSystem(username=username, password=password,

-                             useragent='fedora-business-cards/%s' % __version__)

-         if self.options.username:

-             username = self.options.username

-         userinfo = fas.person_by_username(username)

+         userinfo = self.collect_fas_information()

  

          # set business card fields

          self.fields['name'] = userinfo["human_name"]
@@ -79,7 +58,7 @@ 

          else:

              gpg = "GPG key ID: %s" % userinfo['gpg_keyid']

          self.fields['lines'] = [''] * 6

-         self.fields['lines'][0] = '%s@fedoraproject.org' % username

+         self.fields['lines'][0] = '%s@fedoraproject.org' % userinfo['username']

          self.fields['lines'][1] = 'fedoraproject.org'

          next_line = 2

          if userinfo['ircnick']:
@@ -93,7 +72,8 @@ 

          def cmdline_card_line(data):

              return "| %s%s |" % (data, ' ' * (59 - len(data)))

  

-         done_editing = False

+         # don't prompt user to edit in test mode

+         done_editing = self.options.test

          while not done_editing:

              print("Current business card layout:")

              print("   +" + "-" * 61 + "+")

empty or binary file added
@@ -0,0 +1,47 @@ 

+ #!/usr/bin/python3

+ 

+ import argparse

+ import decimal

+ import os

+ import tempfile

+ import unittest

+ 

+ from .. import common

+ from .. import export

+ from .. import generators

+ 

+ class TestGenerators(unittest.TestCase):

+ 

+     def test_generate_output(self):

+         options = argparse.Namespace(

+             height=decimal.Decimal('2'),

+             width=decimal.Decimal('3.5'),

+             bleed=decimal.Decimal('0'),

+             unit='in',

+             dpi=300,

+             username='',

+             test=True,

+         )

+         # cmyk_pdf currently broken

+         outputs = ['pdf', 'png', 'svg', 'eps']

+         for genstr in generators.__all__:

+             module = common.recursive_import('fedora_business_cards.generators.%s' % genstr)

+             gen = module.generator(options)

+             gen.collect_information()

+             xml = gen.generate_front()

+             with tempfile.TemporaryDirectory() as tmpdirname:

+                 for fmt in outputs:

+                     filename = os.path.join(tmpdirname, 'front.' + fmt)

+                     if fmt == "svg":

+                         export.svg_to_file(xml, filename)

+                     elif fmt == "cmyk_pdf":

+                         export.svg_to_cmyk_pdf(xml, filename, options.height, options.width,

+                                                options.bleed, options.unit, gen.rgb_to_cmyk)

+                     else:

+                         export.svg_to_pdf_png(xml, filename, fmt,

+                                               options.dpi)

+                     self.assertTrue(os.path.exists(filename), filename + " should be generated")

+                     self.assertTrue(os.stat(filename).st_size > 0, filename + " should not be empty")

+ 

+ if __name__ == '__main__':

+     unittest.main()

This implements test mode (so fedora-business-cards can be tested without hitting FAS) in three diffs (the first diff just fixes a typo so is not significant):

  • refactor the extra options parser to be common across all generators
  • add the test mode option
  • implement test mode by refactoring the FAS lookup into a single method that all the collect_information methods make use of

collect_information can be further refactored but that should probably be in a further PR.

Tested by running all three generators in normal, test mode, and test mode with username overridden.

Implements #5

Looks good so far. Can you have a test produce all output formats. I didn't put a ton of detail into #5 - my bad.

My thinking is to have something we can call in a CI system to verify that we get cards as expected. The Inkscape CLI is still not settled and I'd like to stay on top of failures as different Fedora's get different versions.

Also, @salimma - I am going to be away from keyboard for most of next week - so don't let a slow reply bother you please.

@bex sure, I can do that. Two questions then:
- should that be a separate pull request, and we can merge this first?
- is there a preference for which test suite to use (does Fedora projects normally standardize on something e.g. tox?)

I can probably dedicate some time up to verifying that non-empty files are produced, but verifying that we get something graphically meaningful might not be something I'll let someone else tackle.

1 new commit added

  • Test file generation
3 years ago

@bex alright, we now have a test that iterates over all possible output format (except CMYK PDF, will file a bug for it in a bit) and all generators, and verify that they can generate non-empty files.

There is some code duplication, we'll need to refactor the frontend to fix that, but it's probably not a blocker for now.

Instructions in README, but basically python3 -m unittest discover should work (on a system that has fedora-business-cards installed so the dependencies are available).

with a cautionary statement that it is almost midnight ... where are the test files output too? I wanted to review the generator outputs and see if the files "looked" right. Are they saved?

They are currently ephemeral - the test directory gets automatically removed at the end of testing. I can make it output to a known location and preserve the files for further testing, if that's desired?

I'd like that. When I build a new release, I run a copy of each kind of card to verify the output is sane. I don't know of a great way to do that unless we want to embed samples to test for. I have this gut feeling that the outputs are rarely bit-for-bit identical, but I haven't tested that.

Pull-Request has been merged by salimma

3 years ago