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

Remove deprecated Ginkgo options --progress and --slow-spec-threshold #3814

Conversation

SherlockShemol
Copy link
Contributor

@SherlockShemol SherlockShemol commented Nov 11, 2024

When I pr #3811 and I find the E2E tests failed.When I investigate the problem, I find

You're using deprecated Ginkgo functionality:
=============================================
  --progress is deprecated .  The functionality provided by --progress was confusing and is no longer needed.  Use --show-node-events instead to see node entry and exit events included in the timeline of failed and verbose specs.  Or you can run with -vv to always see all node events.  Lastly, --poll-progress-after and the PollProgressAfter decorator now provide a better mechanism for debugging specs that tend to get stuck.
  --slow-spec-threshold is deprecated --slow-spec-threshold has been deprecated and will be removed in a future version of Ginkgo.  This feature has proved to be more noisy than useful.  You can use --poll-progress-after, instead, to get more actionable feedback about potentially slow specs and understand where they might be getting stuck.

This pr is to remove deprecated Ginkgo options --progress and --slow-spec-threshold.

@volcano-sh-bot volcano-sh-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 11, 2024
@SherlockShemol
Copy link
Contributor Author

Copy link
Member

@hwdef hwdef left a comment

Choose a reason for hiding this comment

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

/ok-to-test

@volcano-sh-bot volcano-sh-bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Nov 11, 2024
@hwdef
Copy link
Member

hwdef commented Nov 13, 2024

But there are other errors in CI

@SherlockShemol
Copy link
Contributor Author

SherlockShemol commented Nov 13, 2024

But there are other errors in CI

Could this be due to resource limitations in the CI environment? It seems like the jobs are failing all the time.Or it's because of some resource allocation or scheduling problem.

@hwdef
Copy link
Member

hwdef commented Nov 13, 2024

I'll check this

@JesseStutler
Copy link
Member

Please rebase the new master branch because CI has been fixed: #3817

@SherlockShemol SherlockShemol force-pushed the remove-deprecated-ginkgo-options branch from 06bdf9e to 2f3fd1d Compare November 15, 2024 02:22
@SherlockShemol SherlockShemol force-pushed the remove-deprecated-ginkgo-options branch from 2f3fd1d to f82f82f Compare November 15, 2024 02:34
@SherlockShemol
Copy link
Contributor Author

Please rebase the new master branch because CI has been fixed: #3817

rebased.

@SherlockShemol SherlockShemol requested a review from hwdef November 15, 2024 08:33
@hwdef
Copy link
Member

hwdef commented Nov 18, 2024

Please modify the describe. delete some useless errors. Such as,

  [FAILED] Unexpected error:
      <*errors.errorString | 0xc0005117f0>: 
      [Wait time out]: expected job 'qj-1' to have 2 ready pods, actual got 0
      {
          s: "[Wait time out]: expected job 'qj-1' to have 2 ready pods, actual got 0",
      }
  occurred

@SherlockShemol
Copy link
Contributor Author

Please modify the describe. delete some useless errors. Such as,

  [FAILED] Unexpected error:
      <*errors.errorString | 0xc0005117f0>: 
      [Wait time out]: expected job 'qj-1' to have 2 ready pods, actual got 0
      {
          s: "[Wait time out]: expected job 'qj-1' to have 2 ready pods, actual got 0",
      }
  occurred

modified.

Copy link
Member

@hwdef hwdef left a comment

Choose a reason for hiding this comment

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

/lgtm
/ok-to-test

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2024
@Monokaix
Copy link
Member

Is there some changs of ginkgo output for users when these flags are removed and new flags are added?

@SherlockShemol
Copy link
Contributor Author

Is there some changs of ginkgo output for users when these flags are removed and new flags are added?

 Ginkgo's console output is not guaranteed to be stable for tooling and automation purposes. You should, instead, use Ginkgo's JSON format to build tooling on top of as it has stronger guarantees to be stable from version to version.

Yes, the console output may change and it has stronger guarantees if using the JSON format to build tooling on top of.

@volcano-sh-bot volcano-sh-bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 11, 2024
@volcano-sh-bot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign hwdef
You can assign the PR to them by writing /assign @hwdef in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants