Skip to content

Fix tests not being saved when editing an existing page. #414

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

Closed
wants to merge 2 commits into from
Closed

Fix tests not being saved when editing an existing page. #414

wants to merge 2 commits into from

Conversation

konstantinblaesi
Copy link

@konstantinblaesi konstantinblaesi commented Jul 22, 2017

Fixes #236

Looks like ownership verification is broken. If Name aka author is set your are the owner???

@konstantinblaesi
Copy link
Author

Hi @maxbeatty ,

can you merge this?

@mathiasbynens mathiasbynens requested a review from maxbeatty July 24, 2017 09:56
@maxbeatty
Copy link
Member

@mathiasbynens will you refresh me when a test should be updated?

@konstantinblaesi
Copy link
Author

@maxbeatty Is there anything missing stopping this from being merged?

@maxbeatty
Copy link
Member

@elxa please be patient. I am waiting on Mathias to clarify when a test should be updated. That response will clarify whether this is a change we should make. I will submit a review when I have all of information I need. Thank you for your contribution, but please be patient.

@mathiasbynens
Copy link
Contributor

when [should] a test be updated?

When the test’s owner edits the test while still logged in to the same session (cookie-wise). On the old jsPerf, that used to be an 8-hour window, but that’s an implementation detail.

Copy link
Member

@maxbeatty maxbeatty left a comment

Choose a reason for hiding this comment

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

I don't believe this addresses the root cause of the referenced issue. There's no difference in the session from the first time a page is created to a later date when it is edited which causes tests to always update. A more robust change is needed but I don't fully understand it yet.

@konstantinblaesi konstantinblaesi deleted the fix-issue-236 branch August 16, 2017 20:26
swang added a commit to swang/jsperf.com that referenced this pull request Mar 10, 2018
Fixes jsperf#236

Pretty much similar to jsperf#414, but with an added test to show issue.
swang added a commit to swang/jsperf.com that referenced this pull request Mar 10, 2018
Fixes jsperf#236

Pretty much similar to jsperf#414, but with an added test to show issue.
swang added a commit to swang/jsperf.com that referenced this pull request Mar 10, 2018
Fixes jsperf#236

Pretty much similar to jsperf#414, but with an added test to show issue.
mathiasbynens pushed a commit that referenced this pull request Mar 11, 2018
Fixes #236

Pretty much similar to #414, but with an added test to show issue.
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