Skip to content

Add method modeling flow to test plan#3010

Merged
norascheuch merged 2 commits intomainfrom
nora/modeling-flow-testplan
Oct 25, 2023
Merged

Add method modeling flow to test plan#3010
norascheuch merged 2 commits intomainfrom
nora/modeling-flow-testplan

Conversation

@norascheuch
Copy link
Copy Markdown
Contributor

This PR adds checks for manually testing the new method modeling flow to our release test plan.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@norascheuch norascheuch requested a review from a team as a code owner October 24, 2023 09:56
@norascheuch norascheuch force-pushed the nora/modeling-flow-testplan branch from 9b9ec12 to 7bade3e Compare October 24, 2023 10:04
@norascheuch
Copy link
Copy Markdown
Contributor Author

I wonder how many of these checks should be mandatory, what can be optional.
At the moment we're not testing the method modeling panels multiple method behaviour. Should this be mandatory/optional?

Copy link
Copy Markdown
Member

@koesie10 koesie10 left a comment

Choose a reason for hiding this comment

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

Thanks for adding this! Multiple models are still feature-flagged right now, but since it is going to be removed soon, it's probably fine to not mention that here.

I'm not sure whether these should be optional or not. It's probably good to test them when we have made any changes to them, but don't need to test them when we've only made changes to unrelated code.

Comment thread docs/test-plan.md Outdated
- add a couple of new models for one method
- save and check that the modeling file (use the 'open extension pack' button to open it) shows multiple methods
- check that a 'duplicated classification' error appears in both model editor and modeling panel when a duplicate modeling occurs
- check that a 'conflicting classigication' error appears when a neutral model type is paired with a source kind
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Neutral models aren't conflicting with source kinds only, they are conflicting with models that have the same type as the neutral model's kind. What about:

Suggested change
- check that a 'conflicting classigication' error appears when a neutral model type is paired with a source kind
- check that a 'conflicting classification' error appears when a neutral model type is paired with a model of the same kind

Comment thread docs/test-plan.md Outdated
- check that clicking on the error highlights the correct modeling in both the editor and the modeling panel
3. Check the Method Usage
- Check that the Method Usage Panel opens and jumps to the correct usage when clicking on 'View' in the model editor
- Check that the usages are opening when clicking on a usage
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We had a bug here where the first one would show correctly, but clicking on any other ones would still only show the first one. Would it make sense to change this to something like:

Suggested change
- Check that the usages are opening when clicking on a usage
- Check that the second and third usages show when clicking on them

Copy link
Copy Markdown
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

I agree with Koen's points. But other than that looks good to me

@norascheuch
Copy link
Copy Markdown
Contributor Author

I implemented Koens suggestions and also moved most test cases to optional. In the mandatory testing we now only check that both Panels exist, show the first usage (usage panel) and show and update modeling (modeling panel).

@norascheuch norascheuch force-pushed the nora/modeling-flow-testplan branch from 793667a to 807ac4a Compare October 25, 2023 08:23
Copy link
Copy Markdown
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

There's a tiny linter error, but otherwise the changes LGTM

Thanks for moving stuff into "optional". I think that's the right move for these extra tests of the model editor.

Comment thread docs/test-plan.md Outdated
### Selecting repositories to run on
### Modeling Flow

2. Check that a method can have multiple models:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The linter has spotted this needs to start with 1, though I believe it won't actually affect the rendering it'll be nice to be consistent.

@norascheuch norascheuch force-pushed the nora/modeling-flow-testplan branch from 807ac4a to 0e53067 Compare October 25, 2023 08:57
@norascheuch norascheuch merged commit 3f83027 into main Oct 25, 2023
@norascheuch norascheuch deleted the nora/modeling-flow-testplan branch October 25, 2023 12:29
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.

3 participants