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

Diff wrapper does not show difference between 1.5 and 1.6 #423

Closed
KatieWoe opened this issue Jun 27, 2022 · 10 comments
Closed

Diff wrapper does not show difference between 1.5 and 1.6 #423

KatieWoe opened this issue Jun 27, 2022 · 10 comments

Comments

@KatieWoe
Copy link
Contributor

Test device
Dell
Operating System
Win 11
Browser
Chrome
Problem description
For phetsims/qa#816.
In the diff wrapper for 1.6, entering the url for 1.5 shows no results and the console shows the errors shown bellow.

Visuals
nodiff

@KatieWoe
Copy link
Contributor Author

I got the same error when trying to compare 1.6 to itself.

@KatieWoe
Copy link
Contributor Author

This also happens in Firefox

@samreid
Copy link
Member

samreid commented Jul 6, 2022

I think the user interface was changed so no you are supposed to type "1.5" in the textbox instead of a full URL. Can you please re-test it that way?

@KatieWoe
Copy link
Contributor Author

KatieWoe commented Jul 7, 2022

That does seem to work for the old version. Typing 1.6 to try to compare it to itself does not work though.
I also see this in phetsims/qa#818

@zepumph zepumph assigned samreid and zepumph and unassigned KatieWoe Jul 13, 2022
@zepumph
Copy link
Member

zepumph commented Jul 13, 2022

We will investigate why we can't load 1.6 and compare it to 1.6.

@samreid
Copy link
Member

samreid commented Jul 14, 2022

If I recall correctly, yesterday @zepumph volunteered to check this part. If I misremember or if you want me to take the lead for any reason, please reassign me.

@samreid samreid removed their assignment Jul 14, 2022
@zepumph zepumph assigned samreid and unassigned zepumph Aug 2, 2022
@samreid
Copy link
Member

samreid commented Aug 3, 2022

I believe this answer is straightforward. It is able to load prior versions because the API file for the prior versions is published on the phet-io website at a standard location. Since we are testing a dev or RC version for 1.6, there is no API file for the production version of 1.6 at the standard location, therefore the comparison cannot be made. I believe this is an acceptable and useful constraint that does not need to be documented because it will not impact our clients. Once we have published a production version of 1.6, I would expect it to be able to compare to itself. But we will not be able to test that until we publish a production version.

@zepumph OK to close? Or do you want a spot check after publication?

@zepumph
Copy link
Member

zepumph commented Aug 3, 2022

Recent changes to the diff wrapper have caused us to not be able to just diff two arbitrary links. In my opinion that is a regression that was not necessary, and not that beneficial. I agree that this error is "acceptable" because it won't effect clients, but why would we set up our wrapper to not be able to diff against a version in an RC. It just seems like we are asking to have to do more work later.

@zepumph zepumph assigned samreid and unassigned zepumph Aug 3, 2022
@samreid
Copy link
Member

samreid commented Aug 3, 2022

Client.getAPIFile currently takes a version. Do you recommend that getAPIFile would take version|URL? Or two methods, like getAPIFileForVersion and getAPIFileForURL?

I also tried entering 1.6.0-dev.2 for the comparison and it said APIs are identical. Is that acceptable for now? Is this work blocking the upcoming release?

image

@samreid samreid assigned zepumph and unassigned samreid Aug 3, 2022
@samreid
Copy link
Member

samreid commented Aug 9, 2022

@zepumph and I discussed that we have utilities for comparing API files from specific URLs such as dev versions. We can download them and run grunt compare-phet-io-api or something. This wrapper seems good enough for clients and no more work is needed for this milestone, closing.

@samreid samreid closed this as completed Aug 9, 2022
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