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

Async #378

Merged
merged 59 commits into from
Oct 17, 2023
Merged

Async #378

merged 59 commits into from
Oct 17, 2023

Conversation

StorytellerCZ
Copy link
Member

Continuation of #361

bratelefant and others added 30 commits April 10, 2023 18:14
@bratelefant
add async common methods
fe0811f
@bratelefant
method names
0796250
@bratelefant
buggy Role check fixed
67675d3
@bratelefant
async fixes
4a56f24
@bratelefant
add roles fixes
d1ff02d
@bratelefant
create Role upsert fix
366f4fd
@bratelefant
Merge pull request #1 from Meteor-Community-Packages/master
674957e 
Keep up with master
@bratelefant
re-added fiber version of createIndex
d2fc409
@StorytellerCZ
Merge branch 'master' into master
f549028
@bratelefant
Merge branch 'Meteor-Community-Packages:master' into master
2bbfe4a
@jankapunkt @bratelefant
build(core): add lint script to package.json
6451989
@jankapunkt @bratelefant
fix: standard lint fixed
4c853eb
@jankapunkt @bratelefant
tests: added complete working test and dev infrastructure project
527307c
@jankapunkt @bratelefant
docs: add contribution guidelines and testing guide
42b26bb
@jankapunkt @bratelefant
core: update gitignore
5029e7a
@jankapunkt @bratelefant
core: move package json to testapp
49d6574
@jankapunkt @bratelefant
tests: fix tests to enable complete coverage #report
94566cc
@jankapunkt @bratelefant
core: add .meteorignore for package publishing
494d63b
@jankapunkt @bratelefant
ci: add testsuite
9109a9c
@jankapunkt @bratelefant
core: ignore examples folder on publishing
bfb1cc5
@jankapunkt @bratelefant
ci: fix matrix args
993bae5
@jankapunkt @bratelefant
build(core): move chai dependency to package.json
62b6822
@jankapunkt @bratelefant
fix(tests): use correct path in testapp setup script
cef3778
@jankapunkt @bratelefant
fix(tests/ci): update paths and lint args to run linter
e8a0803
@jankapunkt @bratelefant
ci: fix tests install
eb544d6
@jankapunkt @bratelefant
ci: fix meteor version to 2.12
dea8b43
@jankapunkt @bratelefant
build(core): add lint script to package.json
eaeb5e6
@jankapunkt @bratelefant
fix: standard lint fixed
58cb565
@jankapunkt @bratelefant
tests: added complete working test and dev infrastructure project
9a8e832
@jankapunkt @bratelefant
core: update gitignore
5e22b58
@StorytellerCZ
Copy link
Member Author

Now we mainly need to add test suite for async.

jankapunkt
jankapunkt requested changes Aug 24, 2023
View reviewed changes
Copy link
Member

@jankapunkt jankapunkt left a comment

Choose a reason for hiding this comment

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

somehow we ended up with two meteor projects for testing, someapp and testapp and I think someapp has accidentally been added.

.DS_Store Outdated Show resolved Hide resolved
someapp/packages/roles Outdated Show resolved Hide resolved
@StorytellerCZ
Async client tests
78d4ccf 
Also adjusted Meteor versions for test and package
@StorytellerCZ StorytellerCZ added this to the Release 4.0 milestone Sep 23, 2023
@StorytellerCZ
Copy link
Member Author

OK, figured out the tests, now it is just about converting the test suite to it.

@StorytellerCZ
Copy link
Member Author

StorytellerCZ commented Sep 24, 2023 •
edited
Loading

Got all the async tests running.
I have removed migration tests which I don't feel need to be in the async tests as they should have been run before upgrading to Meteor 3, maybe we should mention that in the readme.

There seems to be a bug in the async version of detecting roles cycles in _addRoleToParentAsync function, which seem to break these tests. Right now I don't see what it could be as the async transition seems to have been done well. Fixing that I think could fix at least half the failing tests. @bratelefant any ideas?

@bratelefant
Copy link
Contributor

Got all the async tests running. I have removed migration tests which I don't feel need to be in the async tests as they should have been run before upgrading to Meteor 3, maybe we should mention that in the readme.

There seems to be a bug in the async version of detecting roles cycles in _addRoleToParentAsync function, which seem to break these tests. Right now I don't see what it could be as the async transition seems to have been done well. Fixing that I think could fix at least half the failing tests. @bratelefant any ideas?

Guess I fixed this, cf #380

Sometimes it's the small things that matter ;)

@jankapunkt
Copy link
Member

Yup, tests are fixed, thanks @bratelefant 🙌

@StorytellerCZ from my end it's good, should we do a final review or do you think it's fine?

@StorytellerCZ
Copy link
Member Author

I think we might need client side tests for async as well, but I think it is fine. I will prepare a feature release once I get some time.

@StorytellerCZ
Copy link
Member Author

@jankapunkt so the client tests were much easier than I thought, honestly I kind of find them lacking, but that is for another time. For me we can merge and release this as a new feature version. I will have to look into re-generating the docs with this.

jankapunkt
jankapunkt approved these changes Oct 14, 2023
View reviewed changes
Copy link
Member

@jankapunkt jankapunkt left a comment

Choose a reason for hiding this comment

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

Ltgm

@StorytellerCZ StorytellerCZ mentioned this pull request Oct 16, 2023
Merged
@StorytellerCZ StorytellerCZ merged commit 5fd6694 into master Oct 17, 2023
@StorytellerCZ StorytellerCZ deleted the feature/async branch May 4, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jankapunkt jankapunkt jankapunkt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
@StorytellerCZ's work
Archived in project
MCP Async/await (Meteor 3) migration
Archived in project
Milestone
Release 4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@StorytellerCZ @bratelefant @jankapunkt