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

test: mock pypandoc.convert_text when testing export views #1117

Closed
wants to merge 3 commits into from

Conversation

afuetterer
Copy link
Member

@afuetterer afuetterer commented Aug 12, 2024

Description

Resolves #1092 and #1113

Tasks

  • add mocked_convert_text fixture, which should be used in combination with export_format fixture, when testing export views
  • add mocked_convert_text to all export view test functions
  • remove special treatment of XML export (e.g. assert root.tag == 'rdmo')
  • add a new parametrized "unit test", that tests all export formats with one input, see test: mock pypandoc.convert_text when testing export views #1117 (comment)
  • test for invalid export format
  • assert status codes
  • assert Content-Type header
  • assert Content-Disposition header
  • rebase branch 2.3.0
  • squash commits

Types of Changes

  • Refactoring (no functional changes, no api changes)

Checklist

@afuetterer

This comment was marked as resolved.

@afuetterer

This comment was marked as resolved.

@jochenklar

This comment was marked as resolved.

@afuetterer
Copy link
Member Author

How should I test render_to_format? With a single project from the database and once for every export format, meaning number of tests = number of export formats.

It could also be number of rdmo models * number of export formats.

Both is better than running pypandoc for each user in multiple parametrized tests.

@jochenklar
Copy link
Member

I think it should suffice to run pandoc with one html output for each format. For some reason render_to_format takes a template and a context (maybe it would be better to have function to render the template and one to convert with pandoc, but this can be done in the future.) I would use the Test project 1 and the project_answers view. So basically this view: https://github.com/rdmorganiser/rdmo/blob/main/rdmo/projects/views/project_answers.py#L60.

@afuetterer

This comment was marked as resolved.

@jochenklar
Copy link
Member

I think of a dedicated test like this:

    project = Project.object.get(1)
    project.catalog.prefetch_elements()

    context = {
        'project': project,
        'project_wrapper': ProjectWrapper(project),
        'title': project.title,
        'format': format,
        'resource_path': get_value_path(project)
    }

    render_to_format(request, format, project.title,
                     'projects/project_answers_export.html', context)

I wonder if we ever would ever look at the outputs. I think just checking if the render_to_format runs without error is enough.

@afuetterer
Copy link
Member Author

I still need to add the "one" test, but the mocking shaved off a couple of minutes of ci runtime. It is now under 30 minutes.


@pytest.mark.django_db()
@pytest.mark.parametrize("export_format", export_formats)
def test_render_to_format(rf, context, export_format):
Copy link
Member Author

Choose a reason for hiding this comment

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

render_to_format, export_format xml and project don't play along. xml was not tested previously. This kind of data can not be exportes as xml?

Is the test still alright?

Copy link
Member

Choose a reason for hiding this comment

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

xml is not using render_to_format so it does not need to be tested here. It should also stay in the regular tests and not be mocked.

Copy link
Member

Choose a reason for hiding this comment

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

Looks fine otherwise, thanks a lot!

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦‍♂️

Okay, thanks for the hint. Only pypandoc.convert_text is mocked.

I removed multiple checks of the generated XML. Should these checks stay then?

Copy link
Member

Choose a reason for hiding this comment

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

yes, the xml serializers/renderers stuff is pretty unique and also fast, please keep it in


@pytest.mark.django_db()
@pytest.mark.parametrize("export_format", export_formats)
def test_render_to_format(rf, context, export_format):
Copy link
Member

Choose a reason for hiding this comment

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

xml is not using render_to_format so it does not need to be tested here. It should also stay in the regular tests and not be mocked.


@pytest.mark.django_db()
@pytest.mark.parametrize("export_format", export_formats)
def test_render_to_format(rf, context, export_format):
Copy link
Member

Choose a reason for hiding this comment

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

Looks fine otherwise, thanks a lot!

@afuetterer
Copy link
Member Author

I am confused. What is the next target branch for PRs?

release-2.3.0 or 2.3.0?

@jochenklar
Copy link
Member

Hi @afuetterer , the new development branch is 2.3.0, I switch branches a lot and I don't want to type release all the time. After the release the tag willbe 2.3.0 I hope this does not cause problems.

@jochenklar jochenklar deleted the branch rdmorganiser:dev-2.2.0 September 20, 2024 08:17
@jochenklar jochenklar closed this Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants