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

What should a phet-io dev test look like? #19

Closed
zepumph opened this issue Jul 6, 2017 · 39 comments
Closed

What should a phet-io dev test look like? #19

zepumph opened this issue Jul 6, 2017 · 39 comments
Assignees

Comments

@zepumph
Copy link
Member

zepumph commented Jul 6, 2017

This should be handled before #18, which is a phet-io dev test. We should solidify what is a minimal QA test that makes us feel good about delivering these versions to close partners. These dev tested sims are intented to be used during development while iterating quickly. Tagging @phet-steele and @ariel-phet to weigh in, but it will probably look similar to the matrix we have for normal phet-io, but for fewer devices.

Also we should think about how much phet brand testing to include in these tests. Ideally a phet-io dev test could stand on its own, and didn't need a phet brand dev test first.

@ariel-phet
Copy link
Contributor

@zepumph @phet-steele my guess is that what the test should look like depends a bit on the client

For instance, if the test is for sonification, do anything but the sonification wrappers need to be tested? Are the clients likely to break features not related to sonification? What platforms are they generally targeting?

If the test is for a data collection study, shouldn't it depend a bit on the nature of the study...ie is it just record and playback? Playing nice with metacog? Does it depend if it is a data collection study etc

@phet-steele
Copy link
Contributor

We should solidify what is a minimal QA test that makes us feel good about delivering these versions to close partners.

@zepumph you make it sound like we would be delivering dev links without an RC test? We shouldn't. Only after dev + RC testing should things be delivered, which is anything but minimal testing.

Anyway, I believe this does come down to the purpose of the sim, which is what @ariel-phet was getting at. If the sim is NOT intended for general use, but has more targeted/lite instrumentation, this should be noted in the dev test. From there, a simple mention of what wrappers and platforms the client would be using would be needed.

If the sim is destined for general use, meaning everything and anything has been instrumented and all wrappers could be used, it might be a good start to test the more comprehensive and widely used wrappers (@zepumph, you could speak to what's "widely used"). This would probably include checking the data stream, record & playback, and maybe the state wrapper, but that's it? I'd imagine just testing on Win Chrome and Mac Safari would be decent coverage before an RC test (iPad 2 isn't so popular with phet-io, right?).

@phet-steele
Copy link
Contributor

Also we should think about how much phet brand testing to include in these tests.

Again this seems situational. It probably depends on how long ago the phet brand sim (after instrumentation) was tested. The longer ago, or if it has never been tested, would call for more phet brand testing. Is not the plain Simulation link in the wrapper index essentially the phet sim? We could use that when more phet brand testing is called for.

@zepumph
Copy link
Member Author

zepumph commented Jul 13, 2017

@zepumph you make it sound like we would be delivering dev links without an RC test? We shouldn't

I think that's what we should try to do, but only for specific cases. We want another type of deployment process, when we are giving to 3rd parties that are in the development process with us. Not for general use, but just to expedite the turnaround and minimize qa. Basically the same reason we have dev testing for the phet brand.

@ariel-phet said

If the test is for a data collection study, shouldn't it depend a bit on the nature of the study...ie is it just record and playback? Playing nice with metacog? Does it depend if it is a data collection study etc

My understanding is that this test would not fall under this category. Instead it would be for when we are sending a link to Ido before this study, to make sure that the phet-io customizations are correct. Figuring out these customizations are often many delivered sims with only small changes to them. It would be best to think of these as 'dev' releases, not needing full testing, but still requiring some since we are delivering to clients outside of phet. I think that anything given to students for the actual study should go through the rc test, which is the way it is currently being done.

Is not the plain Simulation link in the wrapper index essentially the phet sim? We could use that when more phet brand testing is called for.

Yeah, that is a good idea.

@phet-steele
Copy link
Contributor

@zepumph it sounds like we will need different standards for dev tests based on if the sims will:

  • Be delivered right after dev testing
  • Go on to RC testing

@zepumph, can you outline the general scenarios for the two cases? For example, would a dev test in the first case always be targeted/for a study? Would a dev test in the second case ALWAYS be a general purpose release?

@zepumph
Copy link
Member Author

zepumph commented Jul 13, 2017

@phet-steele,

Two types of dev tests definitely sounds reasonable. I would say that in general they would be about the same amount of work/time. I don't see the first test necessarily taking more time, but rather more focused.

I would say that the second (pre rc test) would look a lot like a phet brand dev test, but would hit all of the wrappers in the index. Maybe we should create a phet-io dev matrix. I would defer to your expertise for that.

When testing for immediate delivery, I would love your opinion on it. I think it would be case dependent, as you two have mentioned. A specific wrapper for a study would only need to touch that new wrapper (classroom-activity and lab-book), whereas Georgia Tech may need the whole wrapper suite tested, but potentially only for a single browser.

What do think about a bit of "simulation only" testing no matter what?

@zepumph
Copy link
Member Author

zepumph commented Jul 29, 2017

I think that this issue has been discussed pretty well. @phet-steele how would be best to proceed. Do we want to formalize these two processes, something like a matrix or qa issue template, or should it be up to the phet-io developer to just speak to the needs of the sim testing in the issue?

@phet-steele
Copy link
Contributor

phet-steele commented Aug 7, 2017

Do we want to formalize these two processes, something like a matrix or qa issue template, or should it be up to the phet-io developer to just speak to the needs of the sim testing in the issue?

Maybe a hybrid or something? I don't think it is safe or sustainable to just have a developer speak to the needs "correctly". Maybe a list of info to include (template?) is in order, something like:


If this dev version is planned to go to rc, please include the following in a QA issue:

It is assumed, in this case, that all wrappers will be tested. At least one browser per device listed here will be tested.

If this dev version is planned to be delivered prior to rc, please include the following in a QA issue:

  • Mention the appropriate designers.
  • Note the purpose of this sim (targeted 3rd party release, for a study, etc...).
  • Sim version wrapper index link (Example: http://www.colorado.edu/physics/phet/dev/html/resistance-in-a-wire/1.3.0-phetiodev.9/wrappers/index)
  • A complete checklist of any issues to be verified.
  • A complete checklist of wrappers the client is intending to use/need to be functioning for the client's work.
  • A complete checklist of platforms the client is intending to use.
  • Any relevant deadlines.

@zepumph I'm probably missing something, please review that this at least makes sense. I whipped it up pretty quick.

@zepumph
Copy link
Member Author

zepumph commented Aug 7, 2017

  1. I would say that it is more valuable to include a link to /wrappers/index than the root, but it's up to how you want it for your QA team.
  2. No where is hitting the "plain simulation" part of these tests, but I assume that it is in both of them? I don't know if these templates are more to help the developer include everything necessary, or as a master list for the tester to use to make sure they don't forget anything.

@phet-steele
Copy link
Contributor

I would say that it is more valuable to include a link to /wrappers/index than the root, but it's up to how you want it for your QA team.

You're right. I was thinking this issue was for rc testing when writing that line apparently! (rc tests need the root).

No where is hitting the "plain simulation" part of these tests, but I assume that it is in both of them? I don't know if these templates are more to help the developer include everything necessary, or as a master list for the tester to use to make sure they don't forget anything.

The intention of this template is to make it easier for developers to include info QA needs. I think it is better to have the dev explicitly state to test the plain sim in the second case (in case they don't want QA to test it). The first case includes plain testing in that italicized disclaimer.

@zepumph I fixed the first point you mentioned. I was going to make ONE template with both of these cases and instructions to choose one or the other. @zepumph @ariel-phet if all is good, I'll go ahead with that.

@zepumph
Copy link
Member Author

zepumph commented Aug 7, 2017

Sounds great. Thanks @phet-steele !

@zepumph zepumph removed their assignment Aug 7, 2017
@ariel-phet
Copy link
Contributor

@phet-steele sounds good, please go ahead and make the template.

When you have the markdown file, please put a link to it in this issue and resassign me to review it.

@ariel-phet ariel-phet removed their assignment Aug 30, 2017
@phet-steele phet-steele assigned ghost and unassigned phet-steele May 10, 2018
@jessegreenberg
Copy link
Contributor

I could use a template for for PhET-iO dev testing for phetsims/energy-skate-park-basics#411. @lmulhall-phet what is the status of this issue?

@ghost
Copy link

ghost commented Jun 20, 2018

@jessegreenberg, this issue has been backlogged for quite a while. I'll get a template ready.

@ghost ghost assigned ariel-phet and unassigned ghost Jun 20, 2018
@jonathanolson
Copy link
Contributor

What is up for discussion at dev meeting? Also will we have templates for RC tests (including manual and automated versions)?

@KatieWoe
Copy link
Contributor

We have a testing matrix for phet-io rc, but we don't have a template. Creating one is definitely something we can discuss in the meeting. This is to get everyone on the same page and some final decisions.

@samreid
Copy link
Member

samreid commented Aug 16, 2018

@zepumph @chrisklus and I realized we would like QA to test downloading the example wrapper from the root and make sure it works properly as part of QA testing (both case 1 and case 2).

@samreid
Copy link
Member

samreid commented Aug 16, 2018

Meeting notes: We would like to merge the templates into one master template that covers phet and phet-io brands. @lmulhall-phet can you please develop an initial draft and share it here?

@ariel-phet points out PhET brand testing should be first, if that seems good then can proceed to PhET-iO testing.

@ghost
Copy link

ghost commented Aug 17, 2018

OK, here's our attempt at a "master" dev test template:

https://github.com/phetsims/QA/blob/master/issue-templates/dev-test-template.md

@zepumph
Copy link
Member Author

zepumph commented Aug 17, 2018

Very nice! Thank you so much for the good work. Here are some thoughts:

  • I like how much is hidden behind details, but perhaps QA and others may find it annoying for the general dev test like stuff behind the details tag (if everyone else is onboard I totally am though).
  • I liked that the other template had a legend at the top for what the different syntax meant (like {blarg} and [stuff], can we get that back? I guess it is a matter of deciding if you want more of the "stuff to be filled in" documented with comments, or visible in the rendered issue (meaning devs need to replace it).
  • For a11y dev test, I don't see a link to @jessegreenberg's screen reader test, are we doing that anymore to make sure people understand how screen readers work? Perhaps that is beyond the scope of this template, but still I think there should be something like the following in the Screen Reader section: "This sim supports screen readers. Before continuing please make sure that you are familiar with these, see @KatieWoe for more info" or something.
  • I don't see any mention of "matrix" in the doc, do we not have a matrix for dev releases? I thought we had talked about specific devices for PhET-iO release.
  • For Phet-io test, I would say that it would be rare to have a dev test where we just want a single wrapper rather than the entire index, so the Wrappers section seems redundant to me, maybe a blurb like "Please test all links in the wrapper index."

Again good work. This is very exciting stuff, thanks.

@ghost
Copy link

ghost commented Aug 17, 2018

Thanks for the feedback, @zepumph!

I like how much is hidden behind details, but perhaps QA and others may find it annoying for the general dev test like stuff behind the details tag (if everyone else is onboard I totally am though).

We could do <details open> if people want.

I liked that the other template had a legend at the top for what the different syntax meant (like {blarg} and [stuff], can we get that back?

Yes! I'll try to find it.

For a11y dev test, I don't see a link to @jessegreenberg's screen reader test, are we doing that anymore to make sure people understand how screen readers work? Perhaps that is beyond the scope of this template, but still I think there should be something like the following in the Screen Reader section: "This sim supports screen readers. Before continuing please make sure that you are familiar with these, see @KatieWoe for more info" or something.

Will do.

I don't see any mention of "matrix" in the doc, do we not have a matrix for dev releases? I thought we had talked about specific devices for PhET-iO release.

We don't have a matrix for dev tests, but I can make one. At the very least we should probably have a list of platforms that are the "default" platforms to do dev tests on.

For Phet-io test, I would say that it would be rare to have a dev test where we just want a single wrapper rather than the entire index, so the Wrappers section seems redundant to me, maybe a blurb like "Please test all links in the wrapper index."

I'll get rid of the "Wrappers" h3.

@zepumph
Copy link
Member Author

zepumph commented Aug 17, 2018

We don't have a matrix for dev tests, but I can make one. At the very least we should probably have a list of platforms that are the "default" platforms to do dev tests on.

I don't think a matrix is necessary, I just didn't know. I agree that a device this here could be nice. I defer to @ariel-phet and @KatieWoe (and you of course) as the experts in what the best solution is for informing QA personnel to the needed OS/Device combos for the test.

Everything sounds very nice, thanks.

@KatieWoe
Copy link
Contributor

We could have a list of default devices such as Win 10 all browsers, Mac OS 10.13 all browsers, iPad 11.4, and chromebook. If the developer want's something else it would probably go in the "Focus and Special Instructions" section.

@samreid
Copy link
Member

samreid commented Aug 18, 2018

@lmulhall-phet can you please review phetsims/molarity#49 and add a test like that to the test template?

EDIT: I originally addressed @zepumph and @lmulhall-phet but decided it would be clearer to just address @lmulhall-phet since he is taking the lead on the document.

@KatieWoe
Copy link
Contributor

It has been added to the phet-io section of the dev template. This should also be added to the new RC template when @lmulhall-phet and I are ready to create that.

@ghost
Copy link

ghost commented Aug 21, 2018

Here's the new RC template: https://github.com/phetsims/QA/blob/master/issue-templates/rc-test-template-new.md

Constructive criticism welcome, as always.

@zepumph
Copy link
Member Author

zepumph commented Aug 22, 2018

@jessegreenberg please review the a11y sections in both the dev and rc test templates. I thought they looked pretty good as a start (I'm sure things will come up).

@zepumph
Copy link
Member Author

zepumph commented Aug 22, 2018

Oh sorry! I didn't see #173. Unassigning @jessegreenberg.

@KatieWoe
Copy link
Contributor

If everyone agrees, closing.

@KatieWoe
Copy link
Contributor

KatieWoe commented Sep 12, 2018

Testing something

  • check
  • check

@phet-steele
Copy link
Contributor

phet-steele commented Sep 12, 2018

Look at /issues/186 to see that checkboxes are fine if before a numbered list, but not fine if after. In both cases, I assumed checkboxes are under < details >. I did not bother observing behaviour without the use of < details >.

In the rc-template, there is a numbered list in subsection 0.4 that is causing problems. My test in /issues/186 would suggest this list is the culprit, but I'm not making promises.

@phet-steele phet-steele reopened this Sep 12, 2018
@phet-steele
Copy link
Contributor

@KatieWoe made some edits to the dev and rc templates in 27b0a8d and 8d9ff81 that will hopefully let us check some boxes 🤞. Closing for now until there are problems in the next use of the template 😉.

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

7 participants