chrisspen/django-chroniker

smoke test failures

jayvdb opened this issue · 8 comments

Generated by https://github.com/kamilkijak/django-smoke-tests

ERROR: test_smoke_PUT_admin/chroniker/monitor/^(.+)/run/$ (django_smoke_tests.tests.SmokeTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/local/src/django-smoke-tests/django_smoke_tests/generator.py", line 90, in test
    response = http_method_function(url, {})
  File "/usr/local/lib/python3.6/site-packages/django/test/client.py", line 566, in put
    response = super().put(path, data=data, content_type=content_type, secure=secure, **extra)
  File "/usr/local/lib/python3.6/site-packages/django/test/client.py", line 382, in put
    secure=secure, **extra)
  File "/usr/local/lib/python3.6/site-packages/django/test/client.py", line 422, in generic
    return self.request(**r)
  File "/usr/local/lib/python3.6/site-packages/django/test/client.py", line 503, in request
    raise exc_value
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/usr/local/lib/python3.6/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/lib/python3.6/contextlib.py", line 52, in inner
    return func(*args, **kwds)
  File "/usr/local/lib/python3.6/site-packages/django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/django/contrib/admin/sites.py", line 223, in inner
    return view(request, *args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/chroniker/admin.py", line 661, in run_job_view
    job = Job.objects.get(pk=pk)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/query.py", line 399, in get
    clone = self.filter(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/query.py", line 892, in filter
    return self._filter_or_exclude(False, *args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/query.py", line 910, in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
  File "/usr/local/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1290, in add_q
    clause, _ = self._add_q(q_object, self.used_aliases)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1318, in _add_q
    split_subq=split_subq, simple_col=simple_col,
  File "/usr/local/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1251, in build_filter
    condition = self.build_lookup(lookups, col, value)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1116, in build_lookup
    lookup = lookup_class(lhs, rhs)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/lookups.py", line 20, in __init__
    self.rhs = self.get_prep_lookup()
  File "/usr/local/lib/python3.6/site-packages/django/db/models/lookups.py", line 70, in get_prep_lookup
    return self.lhs.output_field.get_prep_value(self.rhs)
  File "/usr/local/lib/python3.6/site-packages/django/db/models/fields/__init__.py", line 972, in get_prep_value
    return int(value)
ValueError: invalid literal for int() with base 10: '77517069-88ec-46d4-a543-8fd0f2e2a9c1'

I'm not sure what you're reporting here. I don't manage django_smoke_tests, and this seems to be trying to insert a UUID value into Django's standard int pk field, which of course isn't allowed, and isn't something I can fix.

This should return a 4xxx client error, not a 5xx server error.

It doesn't make much difference if a bot encounters a 500 error vs a 404 error. But sure, I guess it wouldn't hurt. The run_job_view it's testing does already throw a 404 error given an invalid pk, but it wasn't testing format or type because the pk value is not user accessible.

I've added django-smoke-tests to the build.

It doesn't make much difference if a bot encounters a 500 error vs a 404 error.

Lots of 404 and the server barely logs an error.
Lots of 500 and server ops team gets pagers.

True. However, if a bot's stuffing garbage into your site, especially on a Django admin page, you probably don't want to ignore that. Silently ignoring the error behind a 404 might not always be the correct solution.

A bot isnt stuffing garbage into the site. The 500 is preventing that. In this case, a 4xx is the correct http response code.

I don't know why you're being so stubbornly pedantic. Yes, it's trying to enter garbage into the site. That's the whole point of a smoke test, and that's why it's entering a UUID into a field it knows should be an integer.

Both error codes are "correct". It's a matter of preference to which one you prefer for your application. If a user is trying to enter garbage into a pk field, you should be aware of that, since it probably means something's either broken elsewhere, or your admin has been compromised.

In any case, the django smoke test project appears to be buggy itself and abandoned, so I'm removing it from the build. It's a good idea, but it's only half-baked and throws too many false positives.