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

Enhance PR Testing Process: Implement Retry Mechanism and Expand /test Command Functionality #17151

Closed
1 task done
NekoAria opened this issue Oct 15, 2024 · 3 comments · Fixed by #17154
Closed
1 task done
Labels
RSS enhancement New feature or request to existing RSS

Comments

@NekoAria
Copy link
Contributor

What feature is it?

Improve the PR testing process, specifically:

  1. Implement a retry mechanism for Puppeteer routes that are likely to time out after 30 seconds.
  2. Enhance the /test command to be applicable for testing changes in PRs, not just the master branch.

What problem does this feature solve?

  1. Currently, Puppeteer routes often time out after 30 seconds, resulting in 503 errors. There are no subsequent attempts to retest after such failures, leading to unreliable test results.
  2. The /test command is currently limited to testing the RSSHub running status of the master branch in the current repository. This doesn't allow for effective testing of changes proposed in PRs.

Additional description

Implementing these features would significantly improve the reliability and efficiency of our testing process.
It would reduce false negatives in our CI pipeline and allow contributors to more effectively test their changes before merging.

This is not a duplicated feature request or new RSS proposal

@NekoAria NekoAria added the RSS enhancement New feature or request to existing RSS label Oct 15, 2024
@TonyRL
Copy link
Collaborator

TonyRL commented Oct 16, 2024

The /test command does not cover PRs since one can trigger it by an empty commit or editting the PR title/description, which IMHO much faster than typing /test and copy pasting the ```routes``` section.

@NekoAria
Copy link
Contributor Author

The /test command does not cover PRs since one can trigger it by an empty commit or editting the PR title/description, which IMHO much faster than typing /test and copy pasting the routes section.

However, this method doesn't support testing specific routes only, such as when previous test results returned 503 errors. Specifically, I'm referring to the situation I encountered in my previous PR: #17131.

Currently, if I only want to test the following routes, it's not possible to do so.

/picuki/profile/stefaniejoosten/tagged
/picuki/profile/stefaniejoosten/tagged/0

@NekoAria
Copy link
Contributor Author

Additionally, this is the testing process I conducted in my forked repository: NekoAria#2.
This should give you a clear understanding of why I believe this feature is meaningful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RSS enhancement New feature or request to existing RSS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants