Skip to content

Rules integration tests #633

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Aug 27, 2019
Merged

Rules integration tests #633

merged 5 commits into from
Aug 27, 2019

Conversation

hiranya911
Copy link
Contributor

@hiranya911 hiranya911 commented Aug 26, 2019 •
edited
Loading

Integration tests that run against the production service:

$ npm run test:integration -- --grep securityRules

> firebase-admin@8.3.0 test:integration ~/Projects/firebase-admin-node/public
> mocha test/integration/*.ts --slow 5000 --timeout 20000 --require ts-node/register "--grep" "securityRules"



  admin.securityRules
    createRulesFileFromSource()
      ✓ creates a RulesFile from the source string
      ✓ creates a RulesFile from the source Buffer
    createRuleset()
      ✓ creates a new Ruleset from a given RulesFile
      ✓ rejects with invalid-argument when the source is invalid
    getRuleset()
      ✓ rejects with not-found when the Ruleset does not exist
      ✓ rejects with invalid-argument when the Ruleset name is invalid
      ✓ resolves with existing Ruleset
    Cloud Firestore
      ✓ getFirestoreRuleset() returns the Ruleset currently in effect
      ✓ releaseFirestoreRulesetFromSource() applies the specified Ruleset to Firestore
    Cloud Storage
      ✓ getStorageRuleset() returns the currently applied Storage rules
      ✓ releaseStorageRulesetFromSource() applies the specified Ruleset to Storage
    listRulesetMetadata()
      ✓ lists all available Rulesets in pages
      ✓ lists the specified number of Rulesets
    deleteRuleset()
      ✓ rejects with not-found when the Ruleset does not exist
      ✓ rejects with invalid-argument when the Ruleset name is invalid
      ✓ deletes existing Ruleset


  16 passing (9s)

ryanpbrewster
ryanpbrewster reviewed Aug 26, 2019
View reviewed changes
Copy link
Contributor

@ryanpbrewster ryanpbrewster left a comment

Choose a reason for hiding this comment

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

tests look okay, they're a bit hard to follow

recommend introducing either:

  • helper methods that keep track of which resources need to be cleaned up
  • make the cleanup remove all rulesets + releases (using relevant ListXyz apis)

@hiranya911
Copy link
Contributor Author

Added some helper functions to improve the readability. PTAL.

ryanpbrewster
ryanpbrewster approved these changes Aug 26, 2019
View reviewed changes
Copy link
Contributor

@ryanpbrewster ryanpbrewster left a comment

Choose a reason for hiding this comment

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

only nits, I still feel like the test cleanup is quite difficult to reason about but it's not worth blocking on

@ryanpbrewster ryanpbrewster removed their assignment Aug 26, 2019
rachelmyers
rachelmyers approved these changes Aug 26, 2019
View reviewed changes
Copy link

@rachelmyers rachelmyers left a comment

Choose a reason for hiding this comment

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

I suggested a rename for tempRulesets and have a question about the last test, but LGTM!

@hiranya911
Copy link
Contributor Author

Renamed tempRulesets to rulesetsToDelete, and added a verification/assertion to the deleteRuleset() test case.

@hiranya911 hiranya911 assigned hiranya911 and unassigned rachelmyers Aug 26, 2019
@hiranya911 hiranya911 merged commit 3eab019 into rules Aug 27, 2019
@hiranya911 hiranya911 deleted the hkj-rules-integ branch August 27, 2019 17:30
hiranya911 added a commit that referenced this pull request Sep 18, 2019
@hiranya911
feat: Security Rules Management API (#645)
0b2082f 
* Implementing the Firebase Security Rules API (#604)

* Implementing the Firebase Security Rules API

* More argument validation and assertions

* Cleaning up the rules impl

* Internal API renamed

* Fixing a typo in a comment

* Implemented createRulesFileFromSource() and createRuleset() APIs (#607)

* Implementing the Firebase Security Rules API

* More argument validation and assertions

* Adding the rest of the CRUD operations for rulesets

* Cleaning up the rules impl

* Cleaned up tests

* Adding some missing comments

* Removing support for multiple rules files in create()

* Implemented the deleteRuleset() API (#609)

* Added deleteRuleset API

* Merged with source

* Implemented the API for releasing rulesets (#610)

* Implemented the API for releasing rulesets

* Removed createRelease logic

* Updated comment

* Added the getStorageRuleset() API (#613)

* Implemented the API for releasing rulesets

* Removed createRelease logic

* Added getStorageRules() API

* Removed some redundant tests

* Implementing the remaining releaseRuleset APIs (#616)

* Implemented the listRulesetMetadata() API (#622)

* Adding the rules API to the public API surface (#625)

* Added rules API to the public admin namespace

* Updated docs

* Addressing comments regarding the d.ts file

* Updated App typings

* Rules integration tests (#633)

* Rules integration tests

* Refactored by adding some helper methods

* Cleaned up some conditionals

* Added verification for deleteRuleset test

* Renamed tempRulesets

* Handling ruleset limit exceeded error (#636)

* Fixing alignment of an annotation

* Updated comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanpbrewster ryanpbrewster ryanpbrewster approved these changes

@rachelmyers rachelmyers rachelmyers approved these changes

Assignees

@hiranya911 hiranya911

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hiranya911 @rachelmyers @ryanpbrewster