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

Fix tests: write runtime db #76

Merged
merged 5 commits into from Jan 15, 2022
Merged

Fix tests: write runtime db #76

merged 5 commits into from Jan 15, 2022

Conversation

tkellogg
Copy link
Owner

Tests broke in #66 but we didn't catch it because our CI/CD pipeline
doesn't run cargo test. This also enables tests on all platforms.

Ping @JakeStanger

closes #72

Tests broke in #66 but we didn't catch it because our CI/CD pipeline
doesn't run cargo test. This also enables tests on all platforms.

closes #72
@JakeStanger
Copy link
Collaborator

JakeStanger commented Jan 14, 2022

Ah whoops, good spot.

How come the tests are succeeding on MacOS but not Linux/Windows? Is there a flaky test in there or is it actually something platform-specific?

@dswij
Copy link
Contributor

dswij commented Jan 15, 2022

I have been experiencing this on my machines. I thought it was something wrong on my side.

@tkellogg
Copy link
Owner Author

Seems to be 2 problems.

  1. During tests, git commands would exit before printing anything if they were unsuccessful. This hid the git commit failure
  2. git commit fails on Linux & Windows if user.name and user.email aren't set

@tkellogg
Copy link
Owner Author

One more thing: Some places weren't calling .unwrap() on GitRepo::git calls. I'm adding those so that we terminate early automatically. An even better fix would be to make git return a Result so that rustc would warn us if we didn't unwrap.

@tkellogg
Copy link
Owner Author

Remaining failure is a race condition. We use Thread::sleep(6), which seems to be not quite long enough sometimes. The fix is to wait only until dura serve starts producing lines of output. I'm not going to fix that yet. I don't have enough time right now. Later.

@tkellogg tkellogg merged commit cd95efc into master Jan 15, 2022
@tkellogg tkellogg deleted the fix-test branch January 15, 2022 16:06
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.

Tests broken
3 participants