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

Trigger sorting for initial sort column #248

Merged
merged 2 commits into from
Nov 4, 2019
Merged

Conversation

wanam
Copy link
Contributor

@wanam wanam commented Oct 22, 2019

Currently it only sets the toggles for initial sort columns without actually doing the sorting.
This fixes #247

Currently it only sets the toggles for initial sort columns without actually doing the sorting.
This fixes pytest-dev#247
@BeyondEvil
Copy link
Contributor

Would you mind adding a test for the sort? @wanam

@wanam
Copy link
Contributor Author

wanam commented Nov 3, 2019

I think it is not necessary to create a new test, it's already covered by this one: https://github.com/pytest-dev/pytest-html/blob/master/testing/test.js#L26

@BeyondEvil
Copy link
Contributor

Hmm... Does the test fail if you revert your change? 🤔

If not, the test needs to be updated right? @wanam

@wanam
Copy link
Contributor Author

wanam commented Nov 3, 2019

The test will still succeed with/without my change, because the 'initial-sort' falg is default set (hard coded here https://github.com/pytest-dev/pytest-html/blob/master/pytest_html/plugin.py#L425) on the first column (test result), if we set this flag for another column, the current test won't pass without my change.

@BeyondEvil
Copy link
Contributor

BeyondEvil commented Nov 3, 2019

@wanam

Ah, I see. Well, I don't want to change the default.

Is it possible to create a new test then based on the scenario in #247 ?

One that will actually fail without your change, without having to change the hard coded default.

@wanam
Copy link
Contributor Author

wanam commented Nov 4, 2019

@BeyondEvil
To do such scenario, we need to clone js_test_report.html with 'initial-sort' flag set for another column, because this flag needs to be set before running the init on main.js.
Is it ok?

@BeyondEvil
Copy link
Contributor

Oh yeah you're right. Sorry about that.

It's not enough to clone it, you'll have to fiddle with the Gruntfile as well, so let's skip it.

Thanks for the PR! 🙏

Copy link
Contributor

@BeyondEvil BeyondEvil left a comment

Choose a reason for hiding this comment

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

LGTM!

@BeyondEvil BeyondEvil merged commit 3e1f812 into pytest-dev:master Nov 4, 2019
@ssbarnea ssbarnea added the bug This issue/PR relates to a bug. label Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

initial-sort flag not working on the columns other than the status result one
3 participants