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

Unit tests for issue #90 #137

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

HanslettTheDev
Copy link
Collaborator

@HanslettTheDev HanslettTheDev commented Sep 11, 2023

Summary

This PR aims to close issue #90. The PR contains unit tests for the following files in formatos module

  • TXT files
  • DBF files
  • JSON files
  • CSV files
  • SQL files
  • XML files

Checklist

  • Classes, Variables, function and methods logic ok
  • Comments written explaining what the code does
  • All python code is PEP8 compliant (run black .)
  • No lint issues (run flake8)
  • Test coverage with pytest implemented

Manual test evidence

(attach command-line examples, execution output & logs, etc.)

@reingart reingart marked this pull request as draft October 1, 2023 02:33
@HanslettTheDev HanslettTheDev marked this pull request as ready for review November 3, 2023 10:58
Copy link
Member

@reingart reingart left a comment

Choose a reason for hiding this comment

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

This PR seems like a nice initial attempt for coverage of this file formats unit tests.

It would be great to have more real tests for TXT and DBF file formats, specially with a real invoice fields and values.

Most of the UT here are pretty simple, and sample files entrada.txt and facturas.json don't seems to be used.
A similar sample file for DBF would be great.
And you should check field and formats like in the CSV file format unit tests.

If you want we can merge this PR and create the more extensive UT in a new PR

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