pantsbuild #general

Channels

welcome

github-notifications

announce

witty-crayon-22786

04/20/2016, 9:45 PM

@billowy-beach-68498: consider posting the code that isn't working to your review.

billowy-beach-68498

04/20/2016, 9:45 PM

billowy-beach-68498

04/20/2016, 9:46 PM

class PantsIgnoreTestBase(ProjectTreeTestBase): def setUp(self): """ API public """ self.base_dir = self.make_base_dir() self.root_dir = os.path.join(self.base_dir, 'root') # make 'root/' self.makedirs('') self.cwd = os.getcwd() os.chdir(self.root_dir) def tearDown(self): """ API public """ os.chdir(self.cwd) self.rmdirs(self.base_dir) def test_ignore_pattern_blank_line(self): self._project_tree = self.mk_project_tree(self.root_dir, [""])

enough-analyst-54434

04/20/2016, 9:47 PM

Re-think @full-kilobyte-93429 's question and check the whole hierarchy for super calls

witty-crayon-22786

04/20/2016, 9:48 PM

@billowy-beach-68498: er, "to your review", ie, on https://rbcommons.com/s/twitter/r/3698/

mammoth-pharmacist-54215

04/20/2016, 9:52 PM

After chatting with @limited-country-20626 it looks like we need to add some public markers to the following. Does anyone have any general objections to any of these items being marked public? pants.backend.codegen.targets.python_thrift_library PythonThriftLibrary pants.backend.jvm.targets.jarable Jarable pants.backend.jvm.targets.jvm_app BundleProps pants.backend.jvm.targets.jvm_app JvmApp pants.backend.jvm.tasks.jar_task JarBuilderTask pants.backend.jvm.tasks.jvm_binary_task JvmBinaryTask pants.backend.jvm.tasks.scala_repl ScalaRepl pants.backend.python.targets.python_binary PythonBinary pants.backend.python.targets.python_requirement_library PythonRequirementLibrary pants.backend.python.targets.python_target PythonTarget pants.base.fingerprint_strategy DefaultFingerprintStrategy pants.base.fingerprint_strategy FingerprintStrategy pants.ivy.ivy Ivy

billowy-beach-68498

04/20/2016, 9:53 PM

@full-kilobyte-93429: are you saying call “super…” in BaseClass.setUp()?

witty-crayon-22786

04/20/2016, 9:54 PM

@mammoth-pharmacist-54215: would probably be easiest to see whether @limited-country-20626 would like to open that review. would be easiest to discuss in context.

billowy-beach-68498

04/20/2016, 9:55 PM

but the problem is BaseClass.setUp() itself is not executed. Or call it somewhere else? I tried to add setUp() in DerivedClass and call super(DerivedClass, self).setUp() in that but it didn’t help

full-kilobyte-93429

04/20/2016, 9:55 PM

DerivedClass

in your example but if

BaseClass

extends further from other class,will need to do the same

witty-crayon-22786

04/20/2016, 9:55 PM

@full-kilobyte-93429: the default is for

super

to be executed, so that's not true.

witty-crayon-22786

04/20/2016, 9:56 PM

@billowy-beach-68498: please post the broken code to the review, and we can take a look in context.

limited-country-20626

04/20/2016, 9:59 PM

I can probably do that - sometime Friday though.

mammoth-pharmacist-54215

04/20/2016, 10:03 PM

sounds good to me

mammoth-pharmacist-54215

04/20/2016, 10:03 PM

thanks @limited-country-20626

witty-crayon-22786

04/20/2016, 10:05 PM

@full-kilobyte-93429: to be clear, doing the following is not necessary:

Copy code

def my_method(self):
  super(MyClass, self).my_method()
  pass

witty-crayon-22786

04/20/2016, 10:05 PM

because if you don't define the method, the definition from the superclass is used

billowy-beach-68498

04/20/2016, 10:06 PM

Copy code

class ProjectTreeTestBase(AbstractClass):

  @abstractmethod
  def mk_project_tree(self, build_root, ignore_patterns=[]):
    """Construct a ProjectTree for the given build_root path."""
    pass

  def make_base_dir(self):
    """
    :API: public
    """
    return tempfile.mkdtemp()

  def fullpath(self, path):
    """
    :API: public
    """
    return os.path.join(self.root_dir, path)

  def makedirs(self, path):
    """
    :API: public
    """
    safe_mkdir(self.fullpath(path))

  def touch(self, path):
    """
    :API: public
    """
    touch(self.fullpath(path))

  def rmdirs(self, path):
    """
    :API: public
    """
    shutil.rmtree(path)

class PantsIgnoreTestBase(ProjectTreeTestBase):

  def setUp(self):
    """
    :API: public
    """
    self.base_dir = self.make_base_dir()
    self.root_dir = os.path.join(self.base_dir, 'root')

    # make 'root/'
    self.makedirs('')

    # make 'root/...'
    self.touch('apple')
    self.touch('orange')
    self.touch('banana')

    # make 'root/fruit/'
    self.makedirs('fruit')

    # make 'root/fruit/...'
    self.touch('fruit/apple')
    self.touch('fruit/orange')
    self.touch('fruit/banana')

    # make 'root/fruit/fruit/'
    self.makedirs('fruit/fruit')

    # make 'root/fruit/fruit/...'
    self.touch('fruit/fruit/apple')
    self.touch('fruit/fruit/orange')
    self.touch('fruit/fruit/banana')

    self.makedirs('grocery')
    self.touch('grocery/fruit')

    self.cwd = os.getcwd()
    os.chdir(self.root_dir)

  def tearDown(self):
    """
    :API: public
    """
    os.chdir(self.cwd)
    self.rmdirs(self.base_dir)

  def test_ignore_pattern_blank_line(self):
    self._project_tree = self.mk_project_tree(self.root_dir, [""])
    files_list = []
    for root, dirs, files in self._project_tree.walk(''):
      for file in files:
        files_list.append(os.path.join(root, file))
        ……..

class FileSystemPantsIgnoreTest(unittest.TestCase, PantsIgnoreTestBase):

  def mk_project_tree(self, build_root, ignore_patterns=[]):
    return FileSystemProjectTree(build_root, ignore_patterns)

full-kilobyte-93429

04/20/2016, 10:07 PM

@rough-minister-58256: https://github.com/pytest-dev/pytest-cov/issues/107 commented before I realized that's not a pants ticket

rough-minister-58256

04/20/2016, 10:07 PM

@billowy-beach-68498: a github gist (https://gist.github.com/) might be easier to pass around

billowy-beach-68498

04/20/2016, 10:07 PM

I believe setUp in unittest.TestCase is used.

full-kilobyte-93429

04/20/2016, 10:08 PM

i can still see the exception from cov_core in

multiprocess

engine test, upgrading to a newer version worked, will send a rb

billowy-beach-68498

04/20/2016, 10:08 PM

https://gist.github.com/JieGhost/07c58285b3c725be82c1f69793666693

billowy-beach-68498

04/20/2016, 10:09 PM

that looks like a cool tool. gist.

rough-minister-58256

04/20/2016, 10:09 PM

my gut feeling here is that this is because there are no

test_*

methods defined on

FilesystemPantsIgnoreTest

rough-minister-58256

04/20/2016, 10:09 PM

thus it thinks theres nothing to test, so doesn’t exec setUp

witty-crayon-22786

04/20/2016, 10:10 PM

@rough-minister-58256: i have a counter example in test_fs.py though.

enough-analyst-54434

04/20/2016, 10:10 PM

For one thing its more than a bit odd to have the

setUp

method in not-a-UnitTest

enough-analyst-54434

04/20/2016, 10:10 PM

Just fix that if you want a fix, and noodle on the math later

rough-minister-58256

04/20/2016, 10:10 PM

(or you could make

PantsIgnoreTestBase

subclass

unittest.TestCase

)