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

Preserve execution order alphabetically by test title #142

Merged
merged 5 commits into from
Aug 7, 2020

Conversation

SimonBaeumer
Copy link
Member

@SimonBaeumer SimonBaeumer commented Jul 22, 2020

Fixes #

Checklist

  • Added unit / integration tests for windows, macOS and Linux?
  • Added a changelog entry in CHANGELOG.md?
  • Updated the documentation (README.md, docs)?
  • Does your change work on Linux, Windows and macOS?

Copy link
Member

@dylanhitt dylanhitt left a comment

Choose a reason for hiding this comment

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

I know it's not ready for review, but I found sometime. Is the plan to make the alphabetical ordering the default order or flag enabled?

@SimonBaeumer
Copy link
Member Author

SimonBaeumer commented Jul 24, 2020

I know it's not ready for review, but I found sometime. Is the plan to make the alphabetical ordering the default order or flag enabled?

The idea is to enable it by default because currently there is no order guaranteed.

@SimonBaeumer
Copy link
Member Author

@dylanhitt review? 👍

@dylanhitt
Copy link
Member

dylanhitt commented Aug 1, 2020

Sure, I'll look at it Sunday 👍

@dylanhitt
Copy link
Member

Reviewing Monday, current get to my computer today. Sorry about that.

@SimonBaeumer
Copy link
Member Author

Reviewing Monday, current get to my computer today. Sorry about that.

No problem, do it when you have time 👍

Copy link
Member

@dylanhitt dylanhitt left a comment

Choose a reason for hiding this comment

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

👍

@dylanhitt
Copy link
Member

The only thing I really have to add is where do we mention "Commander test will execute tests in alphabetical order by default". I was thinking it should go in the commander test cli help text, and maybe a little bit of text under executing.

Hmm maybe a little example showing sequential execution. Something like you mentioned in #128.

001 - it should print hello world:
  order: 1
  command: echo hello world
  stdout: hello world
  exit-code: 0

003 - it should validate exit code:
  order: 2
  command: exit 1
  exit-code: 1

002 - another test:
  order: 2
  command: exit 1
  exit-code: 1

Maybe a full suite example in examples/ could go a long way. Your thoughts?

@SimonBaeumer
Copy link
Member Author

The only thing I really have to add is where do we mention "Commander test will execute tests in alphabetical order by default". I was thinking it should go in the commander test cli help text, and maybe a little bit of text under executing.

Hmm maybe a little example showing sequential execution. Something like you mentioned in #128.

001 - it should print hello world:
  order: 1
  command: echo hello world
  stdout: hello world
  exit-code: 0

003 - it should validate exit code:
  order: 2
  command: exit 1
  exit-code: 1

002 - another test:
  order: 2
  command: exit 1
  exit-code: 1

Maybe a full suite example in examples/ could go a long way. Your thoughts?

Good idea! I will add it to the help and extend the examples.
Anyway I have extending the help command on my list as well.

@SimonBaeumer SimonBaeumer force-pushed the execute-in-alphabetical-order branch from 08425be to ba142f2 Compare August 7, 2020 11:31
@codeclimate
Copy link

codeclimate bot commented Aug 7, 2020

Code Climate has analyzed commit ba142f2 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 92.4% (0.0% change).

View more on Code Climate.

@SimonBaeumer SimonBaeumer merged commit bfa2319 into master Aug 7, 2020
@SimonBaeumer SimonBaeumer deleted the execute-in-alphabetical-order branch August 7, 2020 11:43
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.

2 participants