factory_boy icon indicating copy to clipboard operation
factory_boy copied to clipboard

Add code coverage

Open kingbuzzman opened this issue 2 years ago • 4 comments

When working on the PR "Implements bulk_create for create_batch if available" I found the need to ensure that all the code I wrote was properly tested/covered under tests. Started experimenting and came up with this PR to add coverage when using the CI

kingbuzzman avatar Jun 23 '22 15:06 kingbuzzman

@francoisfreitag @rbarrois What do you guys think about this? .. Also it would be nice if you create a key over at codecov / equivalent to upload all the files there and not have to download them to see them (PS you will only be able to download the files for a day) :/

kingbuzzman avatar Jun 24 '22 14:06 kingbuzzman

In the spirit of openness;

There is an issue that has alluded me, this only happens sometimes, and only when using pypy{3.7, 3.8} + sqlalchemy + sqlite + coverage. I have been able to reproduce this locally too, but haven't understood the cause. There seems to be little to no information about this issue.

It's always the same error in the same placefactory_boy/tests/test_alchemy.py::SQLAlchemyGetOrCreateTests. test_simple_call:

Traceback (most recent call last):
    File "/home/runner/work/factory_boy/factory_boy/tests/test_alchemy.py", line 138, in test_simple_call
      obj1 = WithGetOrCreateFieldFactory(foo='foo1')
    File "/home/runner/work/factory_boy/factory_boy/factory/base.py", line 40, in __call__
      return cls.create(**kwargs)
    File "/home/runner/work/factory_boy/factory_boy/factory/base.py", line 528, in create
      return cls._generate(enums.CREATE_STRATEGY, kwargs)
    File "/home/runner/work/factory_boy/factory_boy/factory/alchemy.py", line 59, in _generate
      return super()._generate(strategy, params)
    File "/home/runner/work/factory_boy/factory_boy/factory/base.py", line 465, in _generate
      return step.build()
    File "/home/runner/work/factory_boy/factory_boy/factory/builder.py", line 267, in build
      kwargs=kwargs,
    File "/home/runner/work/factory_boy/factory_boy/factory/base.py", line 317, in instantiate
      return self.factory._create(model, *args, **kwargs)
    File "/home/runner/work/factory_boy/factory_boy/factory/alchemy.py", line 110, in _create
      return cls._get_or_create(model_class, session, args, kwargs)
    File "/home/runner/work/factory_boy/factory_boy/factory/alchemy.py", line 77, in _get_or_create
      obj = cls._save(model_class, session, args, {**key_fields, **kwargs})
    File "/home/runner/work/factory_boy/factory_boy/factory/alchemy.py", line 122, in _save
      session.commit()
    File "<string>", line 2, in commit
    File "/home/runner/work/factory_boy/factory_boy/.tox/pypy37-django32-mongo-alchemy-sqlite-cov/site-packages/sqlalchemy/orm/session.py", line 1451, in commit
      self._transaction.commit(_to_root=self.future)
.... a lot of sqlalchemy functions in between
    File "/home/runner/work/factory_boy/factory_boy/.tox/pypy37-django32-mongo-alchemy-sqlite-cov/site-packages/sqlalchemy/orm/loading.py", line 151, in <listcomp>
      rows = [proc(row) for row in fetch]
    File "/home/runner/work/factory_boy/factory_boy/.tox/pypy37-django32-mongo-alchemy-sqlite-cov/site-packages/sqlalchemy/orm/loading.py", line [89](https://github.com/FactoryBoy/factory_boy/runs/7043440395?check_suite_focus=true#step:6:92)0, in _instance
      dict_ = instance_dict(instance)

example 3.7 failed example 3.8 failed


Update: Tested it like this

(i=0
while (true); do
    i=$((i+1))
    echo
    echo "running tests $i"'!!'
    echo
    time tox -e pypy37-django32-mongo-alchemy-sqlite-cov
    if [ ! "$?" = "0" ]; then
        printf '\a\n'
        echo "took $i tries"'!!'
        break
    fi
done)

I think i fixed it. It usually failed around 5-10 iterations. I'm by 68 and no crash!! happy dance

kingbuzzman avatar Jun 24 '22 15:06 kingbuzzman

I’m -0 on running coverage with every run of the CI (once per combination in the python / DB matrix IIUC). I personally seldom look at the coverage. I agree it is useful on occasions, but just having the recipe to generate the coverage is enough for me. The rest of the time, it’s making builds slowers and taking up online storage. I can be convinced, but initially don’t think the change is warranted.

The various improvements you’ve suggested are welcome, some where already split out in separate PRs, and the coverage context helps tracking what tests exercise a bit of code. It’ll will soon be pulled out in another PR. :+1:

francoisfreitag avatar Jun 28 '22 18:06 francoisfreitag

@rbarrois Any thoughts on:

  1. measuring coverage systematically when the project CI runs?
  2. upload it to a specialized platform (e.g. codecov) to make it easier to access?

francoisfreitag avatar Jun 28 '22 18:06 francoisfreitag

There is no point having this around

kingbuzzman avatar Jun 23 '23 15:06 kingbuzzman