mozilla / active-data-recipes

A repository of various activedata queries and recipes
Mozilla Public License 2.0
8 stars 24 forks source link

Add a unit test for formatter.py #23

Closed ahal closed 5 years ago

ahal commented 6 years ago

There are a handful of formatters that expect data in a handful of different formats. We should add a pytest test to our test suite to make sure we don't accidentally break one.

Pytest documentation is here: https://docs.pytest.org/en/latest/contents.html

In order to solve this we'll need to create a new test file called test_formatters.py in the test directory. The test should use parametrized inputs (this is a pytest term) and run each input over each formatter and compare to an expected result. Here is an example of a test (in mozilla-central) that does something similar: https://searchfox.org/mozilla-central/source/python/mozlint/test/test_formatters.py

Though in this case, it might be better to parametrize the inputs rather than the formatters (bonus points for parametrizing both!)

The Pytest fixtures/parametrize features can take some getting used to, so I'm hesitant to mark this a "good-first-issue", but I'm happy to help anyone who's up for it work through this.

nadaa commented 6 years ago

hi @ahal, I will start working on this, I know it's challengable but it is also a learning opportunity :)

ahal commented 6 years ago

Perfect! Yes it is a great learning opportunity, writing unittest's (and familiarity with pytest in general) is a great skill to have.

Let me know if you need any help getting started @nadaa

nadaa commented 6 years ago

@ahal, sure I will post any question I have. Thank you!

nadaa commented 5 years ago

Hi @ahal, what test data I can use?

ahal commented 5 years ago

Um, good question! There are two types of data that the table formatter can expect, either a list of lists, or a dict with keys ordered by the names key.

Example data in the first format:

[
    ['Heading A', 'Heading B', 'Heading C'],
    ['Row 1 Column A', 'Row 1 Column B', 'Row 1 Column C'],
    ['Row 2 Column A', 'Row 2 Column B', 'Row 2 Column C'],
]

Example data in the second format:

{
    'names': ['Heading A', 'HeadingB', 'Heading C'],
    'Heading A': ['Row 1 Column A', 'Row 2 Column A'],
    'Heading B': ['Row 1 Column B', 'Row 2 Column B'],
    'Heading C': ['Row 1 Column C', 'Row 2 Column C'],
}

So feel free to use that example data or make up your own. You should have one of each kind though. We can always look into creating more inputs later if we want, but just two basic sets are good enough for now.

klahnakoski commented 5 years ago

ADR (this program) uses the two formats mentioned above by @ahal internally. The system it queries, called ActiveData, produces three formats: list, table, and cube:

list

{"data": [
    {"HeadingA":"row 1 Column A", "HeadingB":"row 1 Column B", "HeadingC":"row 1 Column C"}, 
    {"HeadingA":"row 2 Column A", "HeadingB":"row 2 Column B", "HeadingC":"row 2 Column C"}, 
    {"HeadingA":"row 3 Column A", "HeadingB":"row 3 Column B", "HeadingC":"row 3 Column C"}, 
]}

table

{
    "header":["Heading A", "Heading B", "Heading C"],
    "data":[
        ["Row 1 Column A", "Row 1 Column B", "Row 1 Column C"],
        ["Row 2 Column A", "Row 2 Column B", "Row 2 Column C"]
    ]
}

cube (default)

{
    "data":{
        "Heading A": ["Row 1 Column A", "Row 2 Column A"],
        "Heading B": ["Row 1 Column B", "Row 2 Column B"],
        "Heading C": ["Row 1 Column C", "Row 2 Column C"]
    }
}
nadaa commented 5 years ago

Thanks @ahal & @klahnakoski . I started like this:

Please let me know what do you think, I am still trying to add a fixture to pass formatters to test functions!

ahal commented 5 years ago

Thanks @nadaa that looks perfect!

Once we add tests for different kinds of formatters (e.g the markdown formatter as well), then having all these expected strings and test data live in the test file itself might get messy. So we might want to move them out into some kind of external file (maybe a JSON file?) to keep the test file itself cleaner. But I think you should get what you have landed for now and we look into refactoring it later if it becomes a problem.

nadaa commented 5 years ago

@ahal , thank you. I 've sent a PR, however, there is an encoding issue. Ya, separating data from test will make it nicer, json/yaml can be a choice. I will look at it.

MadinaB commented 5 years ago

Hi @ahal, Hope you are doing great!

Could you please take a look at this try in PR #76?