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

Do not handle Tag elements as list in environment table #281

Closed
wants to merge 6 commits into from

Conversation

werdeil
Copy link
Contributor

@werdeil werdeil commented Mar 11, 2020

Use case: I want to pass a html.div element to the environment table.

With the current code, the element is seen as a list (as Tag inherits from list), and thus enter the list process which remove any html.* formatting.

Tell me if it is not the right solution.

@BeyondEvil
Copy link
Contributor

Could you perhaps share a use-case with some examples? @werdeil

@werdeil
Copy link
Contributor Author

werdeil commented Mar 11, 2020

Could you perhaps share a use-case with some examples? @werdeil

Of course: let say I want to display the plugins line differently.
Currently the line is {"html": "2.1.0", "metadata": "1.8.0"}
I can change the metadata through the metadata plugin's hook to have something prettier and have something like:

html metadata
2.1.0 1.8.0

which would be with the help of py.xml.html module:

html.table([
    html.tr([
        html.th("html"), 
        html.th("metadata")
    ]), 
    html.tr([
        html.td("2.1.0"), 
        html.td("1.8.0")
    ])
])

Current implementation will lead to have the following line in the environment table, which is not I expected:

"[<'tr' tag object 4460167632>, <'tr' tag object 4460169456>]"

@BeyondEvil
Copy link
Contributor

I wonder if the right thing to do here is to break out the environment generation into its own class. Similar to TestResult.

That would make it easier to make modifications to the environment part of the report using hooks and fixtures. Future-proofing it basically.

A little more work, but better in the long run I think.

Would you be up for that?

@werdeil
Copy link
Contributor Author

werdeil commented Mar 11, 2020

I agree it would be better in the long run. I can try to do something but I'll need more time.

As you just released the 2.1.0, I guess I have the time to do it for next version.

@werdeil
Copy link
Contributor Author

werdeil commented Mar 19, 2020

Closing this PR, I'll push a new one with the environment generation class.

@werdeil werdeil closed this Mar 19, 2020
@werdeil werdeil mentioned this pull request Mar 20, 2020
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