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.

Sign up for GitHub

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

Jump to bottom

FR: collapsing sections within the test window #180

Open
gregveres opened this issue Feb 4, 2020 · 12 comments · May be fixed by #183
Open

FR: collapsing sections within the test window #180

gregveres opened this issue Feb 4, 2020 · 12 comments · May be fixed by #183

Comments

@gregveres
Copy link
Contributor

Is this a bug report or a feature request?

Feature request

Version Info

  • Version of Majestic: 1.6.2
  • Version of Jest:
  • Version of Node:
  • Operating System:

Reproduction Repo

I use a lot of describe blocks in my test to group together tests that test a logical area. For instance, I am testing a navbar component right now that changes based on the type of club that user belongs to and the type of user within that club. So the navbar has to display different menu choices based on the type of user logged in. It also has to display different menu choices based on the options configured for the club. The navbar also has to look different based on the branding for each club.

So there are 3 distinct areas of tests that I have in my unit test file. I enclose each of those in a describe block so I have many levels of describe blocks. I would like to provide a way to collapse describe blocks so that I can focus on the tests I am writing. Here is an example,

describe ('test navbar component) {
  describe ('test menus based on user types) {
    describe ('test menu setup for a regular user') {
    }
    describe ('test menu setup for a club administrator') {
    }
  }

 describe ('test menus based on club types) {
   describe('test club with no courts') {
   }
  ...
  }
}

I would like to be able to click on a describe block name and have it toggle collapse / expand of that block of tests.

And i see that the list of tests gets reloaded quite frequently. I would like it to remember the open /closed state across one of the reloads. It doesn't have to remember the state persistently, just while it is running. I think it would get very annoying if I had closed a bunch of describe blocks, then I edited and saved the unit test file and majestic reparsed the file and "forgot" which blocks were closed. If it did that, it would nullify the usefulness of the feature.

Thanks
Greg
@Raathigesh
Copy link
Owner

It's a very handy feature to have. We could maintain a separate client-side state to keep track of the expanded state based on the name of the block (describe/test/it). Do you think you would be willing to contribute to this feature?

@gregveres
Copy link
Contributor Author

Yes I was thinking of taking on the task since I learned a bit about the client during my investigation into the other issue. And I like the tool and expect to keep using it long term.

@Raathigesh
Copy link
Owner

That's awesome. Let me know if I run into any issues so I could help.

@gregveres
Copy link
Contributor Author

@Raathigesh
The TestFileItem has an id that looks something like this: 'U365Jimuvm6kRCwzXKX-r'. Do you know where that is coming from? It seems to be different for the same test each time the page draws.

My test environment has two test files: file 1 and file 2. When I draw the screen for file 1, I get a set of Ids for the describe and it blocks. Then when I switch to file 2 and back to file 1 to draw file 1 again, the ids are all different.

I was hoping that I could use the Id to uniquely identify the test, but it seems that the Id is random. is that coming from graphQL? from the server component somewhere? I don't think it is coming from jest since the test wasn't rerun when I switched between file 1 and file 2.

If you don't know off the top of your head, that's fine. I will investigate. I was just hoping to save a bit of time.

@Raathigesh
Copy link
Owner

Raathigesh commented Feb 5, 2020 •
edited
Loading

The ID is generated every time the file is parsed. And the file is parsed everytime you switch to that file in the UI.

@Raathigesh
Copy link
Owner

I wonder whether we could maintain the collapsed state by creating a clientID in the frontend which is essentially the title of the test concatenated with its parent block title.

  • First Describe (clientID: "First Describe")
  • Second Describe (clientID: "First Describe-Second Describe")
    • Third Describe (clientID: "First Describe-Second Describe-Third Describe")
      • Test A (clientID: "First Describe-Second Describe-Third Describe-Test A")
      • Test B (clientID: "First Describe-Second Describe-Third Describe-Test B")

@gregveres
Copy link
Contributor Author

I got some time this morning to work on this. My thinking is to tackle this on both the front end and back end. Since you pointed out that the backend always regenerates the id every time the file is refreshed, I started there. With the checkin that I just did (new branch), the backend will re-use Ids for the same elements. It keeps track of the ids that have been assigned already. I keep a cache of ids per describe block so searching for an existing Id only has a small set of strings to search through.

The next step will be harder because I am just learning react.
I have a couple of thoughts here. But independant of the approach, I think I need to keep a map of Id => collapse state outside of any UI component. The UI component will update and reference it.

  1. look for a tree control in react and use that to keep track of collapsed / expanded and some how tie into its sense of collapsed to update an independant store of collapsed/expanded state

  2. I suspect this is where redux comes in with global state, but it doesn't seem like you are using it and I dont know redux enough to introduce it. Also that seems like it might be too heavy for what this is.

  3. do something in the existing component and an external id => collapse map. When rendering, look up the map and prune the children if needed.

I should have more time to work on it this weekend.

@gregveres gregveres linked a pull request Feb 9, 2020 that will close this issue
Open
@Raathigesh
Copy link
Owner

Thanks for doing this. I will go through the PR by this week and get back to you.

@IPWright83
Copy link

I don't know how far you got with this - but if expand/collapse functionality existed, you could by default start with all describe blocks collapsed (unless there is a failing test within it). This then allows you to quickly navigate to the tests you're interested in.

@gregveres
Copy link
Contributor Author

@IPWright83 The code is working, Raaghigesh doesn't like the implementation, which I can understand, its his project.

Hmm, I thought I had started the describe blocks as collapsed, but I don't. I also thought I scrolled to the first error, but I don't. I guess it can be improved some more. :)

It isn't quite as straight forward as just collapsing all the describe blocks. I would like the top level describe block to be open if there is only a single top level describe block. My files all have a single describe block with many describe blocks inside. Now that I look at it, I would like the top level describe block to be open and the rest collapsed.

@Raathigesh
Copy link
Owner

Sorry, @gregveres I never got back to your regarding that collapsible PR. I've been not very active on any of my OSS projects recently but I'll get back to it soon :)

@gregveres
Copy link
Contributor Author

Anybody who is interested in this feature can use my fork. I have updated my fork to the latest version (1.7.0)

gregveres pushed a commit to gregveres/majestic that referenced this issue May 15, 2020
changed default collapsed state to closed
57f07aa 
One of the other majestic users commented on the issue asking if the state defaulted
to closed or open. Thinking it through, it should default to closed but the
implementation defaulted to open. So it has now been changed to default to closed
unless there is a failing test. Then it springs open to show the failed test.

The comments were mentioned in Raathigesh#186 but this change is really for Raathigesh#180
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Collapsable ui gregveres/majestic
3 participants
@IPWright83 @Raathigesh @gregveres