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

test: add integration tests for FullAccessKeyFallback #66

Merged
merged 6 commits into from
Jan 17, 2023
Merged

test: add integration tests for FullAccessKeyFallback #66

merged 6 commits into from
Jan 17, 2023

Conversation

mooori
Copy link
Contributor

@mooori mooori commented Jan 16, 2023

Adds integration tests and removes the corresponding unit tests. The background and motivation are described in #51, which does the same for Pausable.

This only ports existing unit tests. We'll probably want to add more tests. Before doing that, I propose to wait for FullAccessKeyFallback changes resulting from audits.

@mooori mooori marked this pull request as ready for review January 16, 2023 19:43
@mooori mooori requested a review from birchmd January 16, 2023 19:44
@mooori
Copy link
Contributor Author

mooori commented Jan 17, 2023

It might take some time for fixes of above issues to land in workspaces, if they are fixed at all. While the issues lead to inefficient and awkward code, I think it’s not a blocker since this code is only available in tests. In that case we could move on with this PR.

WDYT @birchmd?

birchmd
birchmd approved these changes Jan 17, 2023
View reviewed changes
Copy link

@birchmd birchmd left a comment

Choose a reason for hiding this comment

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

Yes, I'm ok with merging this PR. Making issues on the workspaces repo is good enough to make sure we are being good open source citizens.

@mooori mooori merged commit 12ee017 into Near-One:master Jan 17, 2023
@mooori mooori deleted the fa-key-itest branch January 17, 2023 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@birchmd birchmd birchmd approved these changes

@mrLSD mrLSD mrLSD approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@mooori @mrLSD @birchmd