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

A couple thoughts about the QA test template #792

Closed
zepumph opened this issue Mar 30, 2022 · 12 comments
Closed

A couple thoughts about the QA test template #792

zepumph opened this issue Mar 30, 2022 · 12 comments

Comments

@zepumph
Copy link
Member

zepumph commented Mar 30, 2022

Hey! I just made my first QA issue with the new dev test template (#791), and it was AWESOME. Thanks for the hard work there. I have two thoughts about potential improvements.

  1. In the top comment section geared towards the dev, it could be really nice to have a line that is the template for the title of the issue, like:
  • Name the issue like "Dev Test: {{SIM}} {{VERSION}}"

This way, when we fill in all the other templates (with find/replace), this is filled in too, and we can just copy paste this. I mainly like this idea because I feel like everyone does the titles differently, and I never know exactly how QA wants to see them.

  1. I see this line here: Please include (non-screen reader) a11y testing in these records if applicable. I'm not really sure what a11y means anymore, because we have been trying to remove that notions from the features that the a11y team has been creating over the last few years. Do you mean specific features, or do you mean a11y-team-specific testing in some way?

Thanks again for all you hard work!

@KatieWoe
Copy link
Contributor

For 2, we mean things like keyboard navigation should be tested during this stage. This is a note for QA reminding them to include these features in testing rather than put them off, and that checking those boxes means everything has been tested on that platform except screen readers.

@zepumph
Copy link
Member Author

zepumph commented Apr 1, 2022

Can we be explicit then, like "all time minus screen-reader testing." There is a trend amongst the team to try to remove a11y from the project vernacular. Features are features, and there are lots of them (some of them the a11y-team made). This is the same sentiment that I'm sure you are aware of that caused the new templates to just have a bunch of feature checkboxes at the top.

What do you think?

KatieWoe added a commit that referenced this issue Apr 4, 2022
KatieWoe added a commit that referenced this issue Apr 4, 2022
@KatieWoe
Copy link
Contributor

KatieWoe commented Apr 4, 2022

Changes for Number 2 made above

KatieWoe added a commit that referenced this issue Apr 4, 2022
KatieWoe added a commit that referenced this issue Apr 4, 2022
KatieWoe added a commit that referenced this issue Apr 4, 2022
KatieWoe added a commit that referenced this issue Apr 4, 2022
KatieWoe added a commit that referenced this issue Apr 4, 2022
KatieWoe added a commit that referenced this issue Apr 4, 2022
KatieWoe added a commit that referenced this issue Apr 4, 2022
@pixelzoom
Copy link
Contributor

pixelzoom commented Apr 5, 2022

I see that you've added an <h1> tag to all of the templates, like:

<h1>RC Spot Check Test: {{SIM}} {{VERSION}}</h1>

This feels unnecessary and redundant. The name of the sim and version are already in the issue title, and the issue label identifies the type of test. If you want to provide a template for the issue title that we can cut-&-paste, that would be fine. But duplication like this is exactly what we were trying to eliminate.

@KatieWoe
Copy link
Contributor

KatieWoe commented Apr 5, 2022

Undone (didn't link to this issue). I'm not sure what is desired here, specifically or by individuals. If you guys discuss this decide what you want the first request to look like, let me know and I'll implement it. Until then, closing.

@KatieWoe KatieWoe closed this as completed Apr 5, 2022
@pixelzoom
Copy link
Contributor

pixelzoom commented Apr 5, 2022

Let's get a cut-&-paste title into the template.

@KatieWoe do you want the title to include what type of test it is? (spot-check, lite, etc.)

This could be as easy at converting this:

<h1>RC Spot Check Test: {{SIM}} {{VERSION}}</h1>

to an instruction in the "DEVELOPERS" comment:

use issue title = RC Spot Check Test: {{SIM}} {{VERSION}}

@KatieWoe
Copy link
Contributor

KatieWoe commented Apr 5, 2022

I don't mind either way. I do want it to say RC, RC Spot Check, etc. But sim and version I'm willing to do whatever works best for everyone. But I'm not sure what that is and don't want to make a decision that not everyone agrees with

@pixelzoom
Copy link
Contributor

@KatieWoe said:

Undone (didn't link to this issue). ...

I linked them above.

@pixelzoom
Copy link
Contributor

pixelzoom commented Apr 5, 2022

Why do you need an <h1> header that says "RC Spot Check" if the issue title is "RC Spot Check: Geometric Optics 1.1.0-rc.3", and the issue is labeled rc-spot-check ?

@KatieWoe
Copy link
Contributor

KatieWoe commented Apr 5, 2022

It's easier to see at a glance and less likely to be changed or omitted.

pixelzoom added a commit that referenced this issue Apr 5, 2022
pixelzoom added a commit that referenced this issue Apr 5, 2022
pixelzoom added a commit that referenced this issue Apr 5, 2022
pixelzoom added a commit that referenced this issue Apr 5, 2022
pixelzoom added a commit that referenced this issue Apr 5, 2022
pixelzoom added a commit that referenced this issue Apr 5, 2022
pixelzoom added a commit that referenced this issue Apr 5, 2022
pixelzoom added a commit that referenced this issue Apr 5, 2022
pixelzoom added a commit that referenced this issue Apr 5, 2022
@pixelzoom
Copy link
Contributor

@KatieWoe and I discussed on Zoom.

She would like a header on the body of the issue, because her eye goes to the body, not the title.

In the above commits, I've added a title template (what @zepumph requested in 1. above) to the "DEVELOPER" comment block. For example:

<!---
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

DEVELOPERS: 

* title for this issue = Dev-Lite Test: {{SIM}} {{VERSION}}
* Fill in the {{SIM}}, {{VERSION}}, and {{GITHUB_ISSUE_LINK}} placeholders.
* Delete things that are not relevant, e.g. PhET-iO links for non-PhET-iO tests.

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-->

The title prefix matches the <h1> in the issue body. So for the example above, the <h1> is:

<h1>Dev-Lite Test</h1>

Here are the titles for the 7 templates:

screenshot_1643

@KatieWoe please review, and feel free to close. Also feel free to change the titles or <h1> tags, but please make sure that they match.

@pixelzoom
Copy link
Contributor

@KatieWoe can this issue be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants