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

testbot + group non-atomic tests #76

Closed
wants to merge 4 commits into from
Closed

Conversation

mixmix
Copy link
Member

@mixmix mixmix commented Sep 21, 2022

I noticed we weren't using the testbot everywhere. Changed that to main maintenance easier.
I also considered making all test atomic so the repo was consistent, but the remaining tests were too fidey to do this with - instead I just modified the structure to make it very clear that tests depend on one another

  • left a comment
  • used t.test to provide a clear scope
    • this is needed so that you can have t.only work because starting sbots in the global scope means t.only was never exiting 😠


let tombstoneMsg
let addMsg
// NOTE - these subtests are not "atomic" - db state is shared between them
Copy link
Member Author

Choose a reason for hiding this comment

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

This diff is annoying. I didn't touch the tests at all other then indenting them, and changing to t.test

Note the shared variables and sbot are now "inside" a test scope (not global in the file)

Base automatically changed from atomic-tests-cb to master September 21, 2022 07:53
@staltz
Copy link
Member

staltz commented Sep 21, 2022

I'm confused with what happened to all these PRs. I approved my PR-of-your-PR, but apparently you had changed its base to become a PR-to-master, and now we have these commits in master branch:

But then this PR also has those two commits, plus a git conflict. Also PR #75 has a git conflict. And why was #73 closed?

What's going on??

@mixmix mixmix closed this Sep 27, 2022
@staltz staltz deleted the atomic-tests-cb-testbot branch September 27, 2022 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants