Skip to content

Conversation

@swang
Copy link
Contributor

@swang swang commented Mar 10, 2018

Fixes #236

Pretty much similar to #414, but with an added test to show the issue.

Based on what @mathiasbynens said in that PR #414 (comment)

It seems like the original fix in #414 was correct. I have also written a test that shows the correct action.

If you are the owner of the test (aka you own the 1st revision), only you (with the isOwn flag set to true) should be able to "update" an existing test. Otherwise you are not the owner and you should only be inserting your test cases into a revision.

The reason for all the tests having "empty" test cases is because the code would fall into the UPDATE sql statement every single time regardless of whether you're the actual owner or not because each test always passes a testID unless a new test case is added to your revision of the test.
E.g. every test has the two basic test cases, and so if you try to create a new revision and only edit those 2 test cases, you'll get a blank test revision. If you add an extra test case in that revision, then that one won't have a testID and thus will make an INSERT call and this new revision will only have the new test case.

The additional test case shows that if you are the owner, you make updates if the test cases exist, and if you are not the owner you only make inserts.

Fixes jsperf#236

Pretty much similar to jsperf#414, but with an added test to show issue.
@mathiasbynens mathiasbynens merged commit f144e66 into jsperf:master Mar 11, 2018
@mathiasbynens
Copy link
Contributor

Thank you!

@CTimmerman
Copy link

Still not working on https://jsperf.com/arr-map-filter-reduce/4/edit - Only my added test is inserted and the others are lost, even when updated.

@bthallion
Copy link

I'm having the same issue, editing a test suite and not having the exisiting tests added, for this suite https://jsperf.com/preallocating-array

maxbeatty added a commit that referenced this pull request Oct 9, 2018
* master:
  Adding X-UA-Compatible in the head (#471)
  fix: .snyk & package.json to reduce vulnerabilities (#477)
  add link to wiki (#466)
  Fix issue with bad updates causing disappearing tests (#464)
  Fix cache start (#453)
  Ensure that edited test cases can be set as synchronous/asynchronous 
(#451)
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.

Some tests are not getting posted properly

4 participants