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.

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

E2E test for tokens only namespaces #919

Merged
merged 7 commits into from
Aug 4, 2022
Merged

E2E test for tokens only namespaces #919

merged 7 commits into from
Aug 4, 2022

Conversation

shorsher
Copy link
Member

@shorsher shorsher commented Aug 2, 2022

  • Adds an E2E test for namespaces with only a tokens plugin
  • Moves keynormalization config under namespaces.predefined[].assets.manager.keynormalization
  • Adds common for code adding namespaces, resetting firefly, etc. to a common package

shorsher added 3 commits August 2, 2022 11:11
@shorsher
move keynormalization config under namespaces
1c4824f 
Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
@shorsher
add e2e tests for tokens-only namespaces
ba790af 
Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
@shorsher
commonize e2e code
5b8da24 
moving code for adding namespaces to config, resetting ff, etc. into a
common package

Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
@shorsher shorsher changed the title E2E test for tokens only namespaces, E2E test for tokens only namespaces Aug 2, 2022
@shorsher shorsher mentioned this pull request Aug 2, 2022
Merged
@shorsher
update docs
39a8662 
Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2022 •
edited
Loading

Codecov Report

Merging #919 (dbca11b) into main (3ae7ba8) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head dbca11b differs from pull request most recent head 7c839e1. Consider uploading reports for the commit 7c839e1 to get more accurate results

@@            Coverage Diff             @@
##             main     #919      +/-   ##
==========================================
- Coverage   99.97%   99.95%   -0.02%     
==========================================
  Files         299      299              
  Lines       19475    19481       +6     
==========================================
+ Hits        19470    19473       +3     
- Misses          4        6       +2     
- Partials        1        2       +1
Impacted Files Coverage Δ
internal/coreconfig/coreconfig.go 100.00% <ø> (ø)
internal/identity/identitymanager.go 100.00% <ø> (ø)
internal/assets/manager.go 100.00% <100.00%> (ø)
internal/namespace/config.go 100.00% <100.00%> (ø)
internal/namespace/manager.go 100.00% <100.00%> (ø)
internal/orchestrator/orchestrator.go 100.00% <100.00%> (ø)
internal/tokens/fftokens/fftokens.go 98.84% <0.00%> (-0.70%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@shorsher
update namespace assets key to match global key
c5e24a1 
Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
@shorsher shorsher force-pushed the tokens-only-e2e branch 2 times, most recently from 2a71a09 to 4ac033b Compare August 3, 2022 02:28
@shorsher
Copy link
Member Author

shorsher commented Aug 3, 2022

Adding the migration_consideration label because asset.manager.keynormalization config has been moved under namespaces.predefined[].asset.manager.keynormalization

awrichar
awrichar reviewed Aug 3, 2022
View reviewed changes
docs/reference/config.md Outdated Show resolved Hide resolved
awrichar
awrichar reviewed Aug 3, 2022
View reviewed changes
test/e2e/gateway/common.go Outdated Show resolved Hide resolved
awrichar
awrichar reviewed Aug 3, 2022
View reviewed changes
test/e2e/gateway/tokens_only.go
suite.connector = stack.TokenProviders[0]
suite.db = stack.Database
account := stackState.Accounts[0].(map[string]interface{})
suite.key = account["address"].(string)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to extract this rather than just using the default key?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking through the code, is there any better way to get the defaultkey? I don't see it used anywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to just leave key blank and asset manager will always use the default.

awrichar
awrichar approved these changes Aug 3, 2022
View reviewed changes
Copy link
Contributor

@awrichar awrichar left a comment

Choose a reason for hiding this comment

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

Left a few more questions/comments, but putting an approval on this to merge after you go through them.

@shorsher
add additional gateway testState functionality
dbca11b 
Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
@shorsher
update docs to use correct namespaces config
7c839e1 
 - updating all references of `namespace` to now be `namespaces`

Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
@shorsher shorsher merged commit 050645e into hyperledger:main Aug 4, 2022
@shorsher shorsher deleted the tokens-only-e2e branch August 4, 2022 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@awrichar awrichar awrichar approved these changes

@peterbroadhurst peterbroadhurst Awaiting requested review from peterbroadhurst

@nguyer nguyer Awaiting requested review from nguyer

@nickgaski nickgaski Awaiting requested review from nickgaski

Assignees
No one assigned
Labels
migration_consideration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@shorsher @codecov-commenter @awrichar