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

towards sorting on test state #129

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ktomkeys
Copy link

Towards addressing #103

This would a allow a minimal capability of sorting by failed tests first.

@matepek
Copy link
Contributor

matepek commented May 14, 2020

Hello,

I'm not the author, but let me give some feedback. Your feature could be orthogonal with the sorting. It could be a separate boolean instead of a new enum. Because the remaining tests now are sorted by label but someone might don't want that.

@Trass3r
Copy link
Contributor

Trass3r commented Aug 25, 2020

Otoh you usually want to sort by test state to focus on the failing ones and ignore the rest.

@Trass3r
Copy link
Contributor

Trass3r commented Sep 14, 2020

I added Trass3r@e3aaad4, works perfectly.

@Trass3r
Copy link
Contributor

Trass3r commented Jan 26, 2021

Rebased.
@hbenl thoughts on this?

@krlmlr
Copy link

krlmlr commented Mar 4, 2021

I installed the debug version of the extension (after merging the main branch into it). The sorting works, however the list is not re-sorted after a test run. When I re-sort manually, it works. Ideally, the failing tests would automatically float to the top after each test run.

I second @matepek's suggestion to make this a boolean option, even defaulting to true maybe: we should be able to sort by failure state, then by label or by location or by whatever the user chooses.

@krlmlr
Copy link

krlmlr commented Mar 5, 2021

The list is eventually re-sorted, just not after a failing test run. I'm using https://github.com/meakbiyik/vscode-r-test-adapter to drive the tests.

@hbenl
Copy link
Owner

hbenl commented Mar 17, 2021

  • Like @matepek and @krlmlr I also think this should be a flag
  • The test tree also needs to be resorted when the test states are retired or reset
  • Resorting the tree at the end of a test run and after retiring/resetting the states should only be done when sorting by state is enabled - otherwise it is unnecessary and may degrade performance with large test trees
  • Before resorting the tree (after a test run or after retiring/resetting the states) you need to make sure the suites' states are up to date (they are only recomputed on demand for performance reasons) - to do that, call recalcState() on the TestCollection

I currently don't have time to work on this, but if someone addresses these points, I'd gladly merge this 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.

5 participants