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

Fix session expiration test #7208

Merged
merged 3 commits into from
Feb 19, 2021
Merged

Conversation

davimacedo
Copy link
Member

New Pull Request Checklist

  • [ X ] I am not disclosing a vulnerability.
  • [ X ] I am creating this PR in reference to an issue.

Issue Description

The session is flaky because a session is created and then the expiration date (that parse server created) is compared to a time got from the beginning of the test + the expiration span. Even if it is very fast, it can happen the two dates having different total minutes.

Related issue: #7180

Approach

I changed the test to check if the difference between the two dates is less then or equal to the jasmine test timeout limit.

TODOs before merging

  • Add test cases
  • Add entry to changelog
  • Add changes to documentation (guides, repository pages, in-code descriptions)
  • Add security check
  • Add new Parse Error codes to Parse JS SDK
  • ...

@codecov
Copy link

codecov bot commented Feb 19, 2021

Codecov Report

Merging #7208 (ff6f80a) into master (603cc1f) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7208      +/-   ##
==========================================
- Coverage   94.03%   94.01%   -0.02%     
==========================================
  Files         172      172              
  Lines       12867    12867              
==========================================
- Hits        12099    12097       -2     
- Misses        768      770       +2     
Impacted Files Coverage Δ
src/ParseServerRESTController.js 97.01% <0.00%> (-1.50%) ⬇️
src/Adapters/Files/GridFSBucketAdapter.js 79.50% <0.00%> (-0.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 603cc1f...e97d8e5. Read the comment docs.

@mtrezza
Copy link
Member

mtrezza commented Feb 19, 2021

The date comparison in test default session length looks similarly strange. Do you think it should be changed too?

@davimacedo
Copy link
Member Author

Thanks for the heads up. I haven't noticed the other one.

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

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

LGTM!

@dplewis dplewis merged commit fba096a into parse-community:master Feb 19, 2021
@mtrezza
Copy link
Member

mtrezza commented Feb 19, 2021

Thanks for the heads up. I haven't noticed the other one.

@davimacedo Is that something you wanted to change before merging?

@dplewis
Copy link
Member

dplewis commented Feb 19, 2021

@mtrezza It was added already.

@mtrezza
Copy link
Member

mtrezza commented Feb 19, 2021

Oh right, didn't notice that one.

@davimacedo davimacedo mentioned this pull request Feb 20, 2021
4 tasks
dplewis pushed a commit that referenced this pull request Feb 21, 2021
* Fix session expiration test

* Fix the other test with similar issue

* Remove fit
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Nov 1, 2021
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.0.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants