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

Testament - implement --retry and fix --failing #96

Merged
merged 1 commit into from
Dec 9, 2021
Merged

Testament - implement --retry and fix --failing #96

merged 1 commit into from
Dec 9, 2021

Conversation

shayanhabibi
Copy link
Collaborator

@shayanhabibi shayanhabibi commented Dec 6, 2021 •
edited
Loading

  • Documentation
  • Make easier to maintain and better file structure
  • Make --failing also filter stdout

@shayanhabibi shayanhabibi marked this pull request as ready for review December 6, 2021 03:50
alaviss
alaviss reviewed Dec 6, 2021
View reviewed changes
testament/testament.nim Outdated Show resolved Hide resolved
saem
saem requested changes Dec 6, 2021
View reviewed changes
Copy link
Collaborator

@saem saem left a comment

Choose a reason for hiding this comment

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

Suggested a few things.

testament/categories.nim Outdated Show resolved Hide resolved
testament/specs.nim Outdated Show resolved Hide resolved
@saem
Copy link
Collaborator

saem commented Dec 7, 2021

Your code is so much better. 😭

Ok so, is shrink it to get your retry bits in and maybe have a draft for the additional changes. Imma take a crack at "testament2" this weekend. Basically spec parsing only, shrink the category madness, and ditch megatest.

Should give a much cleaner base to work with... seriously.

saem
saem reviewed Dec 7, 2021
View reviewed changes
Copy link
Collaborator

@saem saem left a comment

Choose a reason for hiding this comment

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

Non-blockers, just noticed something.

testament/specs.nim Outdated
@@ -265,6 +273,10 @@ proc isCurrentBatch*(testamentData: TestamentData; filename: string): bool =

proc parseSpec*(filename: string): TSpec =
result.file = filename
if retryContainer.retry and retryContainer.names.anyIt(it in result.file):
Copy link
Collaborator

Choose a reason for hiding this comment

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

So here is an idea, that you'll love.... Ok maybe love is a strong word, but if it's possible to store the full log, we could see which tests are newly discovered and need to be run, or only store successes?

So some failures, add tests, retry runs failure and new.

testament/testament.nim Outdated Show resolved Hide resolved
@shayanhabibi
Copy link
Collaborator Author

So here is an idea, that you'll love.... Ok maybe love is a strong word, but if it's possible to store the full log, we could see which tests are newly discovered and need to be run, or only store successes?

I like this. Can add this after; I just really want to be able to use the failing/retry when working with other branches from devel.

@shayanhabibi shayanhabibi changed the title [WIP/Help Wanted] Testament workup Implement --retry and fix --failing Dec 7, 2021
@shayanhabibi shayanhabibi changed the title Implement --retry and fix --failing [TESTAMENT] Implement --retry and fix --failing Dec 7, 2021
@shayanhabibi
Copy link
Collaborator Author

shayanhabibi commented Dec 7, 2021 •
edited
Loading

I am happy with the current implementation of --failing and --retry.

It's still not neat enough and there are things to add as saem has suggested, however I think improving compiler tooling is pretty important.

In essence:

  • --failing will now filter stdout and only print failed tests
  • --retry will only run previously failed tests from last all run

@shayanhabibi shayanhabibi reopened this Dec 7, 2021
alaviss
alaviss requested changes Dec 7, 2021
View reviewed changes
testament/testament.nim Outdated Show resolved Hide resolved
testament/backend.nim Outdated Show resolved Hide resolved
testament/backend.nim Outdated Show resolved Hide resolved
testament/backend.nim Outdated Show resolved Hide resolved
testament/backend.nim Outdated Show resolved Hide resolved
testament/backend.nim Outdated Show resolved Hide resolved
testament/backend.nim Outdated Show resolved Hide resolved
testament/backend.nim Outdated Show resolved Hide resolved
testament/specs.nim Show resolved Hide resolved
testament/testament.nim Show resolved Hide resolved
@saem
Copy link
Collaborator

saem commented Dec 8, 2021

This is better, I can make a bunch of other improvements in the weekend, we should be in good shape.

alaviss
alaviss reviewed Dec 8, 2021
View reviewed changes
testament/backend.nim Show resolved Hide resolved
testament/testament.nim Outdated Show resolved Hide resolved
@shayanhabibi shayanhabibi force-pushed the shayanhabibi/testamentCleaning branch 2 times, most recently from fd5b686 to 6a90d6a Compare December 9, 2021 02:52
alaviss
alaviss approved these changes Dec 9, 2021
View reviewed changes
Copy link
Contributor

@alaviss alaviss left a comment

Choose a reason for hiding this comment

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

[TESTAMENT] -> testament: is the only nit I have left.

Great work!

@shayanhabibi shayanhabibi changed the title [TESTAMENT] Implement --retry and fix --failing Testament - implement --retry and fix --failing Dec 9, 2021
alaviss
alaviss reviewed Dec 9, 2021
View reviewed changes
testament/testament.nim Outdated Show resolved Hide resolved
@shayanhabibi
Implement retry & failing
d978bd2 
remove dirty template

parser in template

Comments and style as per @alaviss

documentation and personal style preferences

predecate styleechos with optFailing check

introduce techo to help in the interim

Changed techo to msg; added cache failed results

extract retries

lost

still confsed

retry implementation

reduce msg proc branching
@alaviss
Copy link
Contributor

alaviss commented Dec 9, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 9, 2021

Build succeeded:

@bors bors bot merged commit 8159bd8 into devel Dec 9, 2021
@bors bors bot deleted the shayanhabibi/testamentCleaning branch December 9, 2021 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saem saem saem approved these changes

@alaviss alaviss alaviss approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Test suite reorganization and cleanup
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@shayanhabibi @saem @alaviss @haxscramper