Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add code coverage #946

Closed
wants to merge 73 commits into from
Closed

Conversation

kingbuzzman
Copy link
Contributor

@kingbuzzman kingbuzzman commented Jun 23, 2022

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
Copy link
Contributor Author

kingbuzzman commented Jun 24, 2022

@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) :/

Copy link
Contributor Author

@kingbuzzman kingbuzzman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LMKWYT

- "3.10"
- "pypy-3.7"
- "pypy-3.8"
- ["3.7", "py37"]
Copy link
Contributor Author

@kingbuzzman kingbuzzman Jun 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds pretty names.. for the coverage filename. If we move this to codecov / alternative, there would be no need to have this around.

if: matrix.run-coverage == 'yes'
uses: actions/upload-artifact@v3
with:
name: factory_coverage_${{ matrix.python-version[1] }}_${{ matrix.database-type }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to move this over to a codecov / equivalent and not have to download it..

Makefile Outdated
@@ -3,9 +3,21 @@ TESTS_DIR=tests
DOC_DIR=docs
EXAMPLES_DIR=examples
SETUP_PY=setup.py
PYTHON_TEST=python \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this here to reuse it for coverage support

Makefile Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
postgres: DJANGO_SETTINGS_MODULE=tests.djapp.settings_pg

whitelist_externals = make
commands = make test
commands =
!cov: make test
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't want to force anyone to use nocov, this way its more "intelligent", even though there is a bit of "magic".

@kingbuzzman
Copy link
Contributor Author

kingbuzzman commented Jun 24, 2022

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

Makefile Outdated
@rm -rf .coverage htmlcov/

coverage-test:
$(PYTHON_ERROR_ON_WARN) $(COVERAGE_PATH) run -m unittest
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rbarrois friendly bump :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rbarrois friendly bump :)

@kingbuzzman
Copy link
Contributor Author

There is no point having this around

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants