ladjs / superagent

Ajax for Node.js and browsers (JS HTTP client). Maintained for @forwardemail, @ladjs, @spamscanner, @breejs, @cabinjs, and @lassjs.
https://ladjs.github.io/superagent/
MIT License
16.58k stars 1.33k forks source link

Feature/GitHub action improve #1717

Closed yunnysunny closed 2 years ago

yunnysunny commented 2 years ago
  1. Tigger CI on push and pull request event.
  2. Replace the config of sauce with github action secret enviroment variables.
codecov-commenter commented 2 years ago

Codecov Report

Merging #1717 (b2b330b) into master (633e467) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1717   +/-   ##
=======================================
  Coverage   86.67%   86.67%           
=======================================
  Files          14       14           
  Lines        1133     1133           
=======================================
  Hits          982      982           
  Misses        151      151           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 633e467...b2b330b. Read the comment docs.

seancolyer commented 2 years ago

I don't know this project well, but I see the timeout bump in #1716. It looks though like this value is overridden in the vast majority of these tests (including the one that fails here if I'm reading correctly). You may need to do something like the following instead:

diff --git a/test/node/query.js b/test/node/query.js
index 2fee688..ae49f39 100644
--- a/test/node/query.js
+++ b/test/node/query.js
@@ -210,5 +210,5 @@ describe('req.query(Object)', () => {
     });

     stream.pipe(request_);
-  });
+  }).timeout(15000);
 });

Again, not super sure of conventions (e.g. repo seems to use a lot of 15_000 vs 15000)

yunnysunny commented 2 years ago

I don't know this project well, but I see the timeout bump in #1716. It looks though like this value is overridden in the vast majority of these tests (including the one that fails here if I'm reading correctly). You may need to do something like the following instead:

diff --git a/test/node/query.js b/test/node/query.js
index 2fee688..ae49f39 100644
--- a/test/node/query.js
+++ b/test/node/query.js
@@ -210,5 +210,5 @@ describe('req.query(Object)', () => {
     });

     stream.pipe(request_);
-  });
+  }).timeout(15000);
 });

Again, not super sure of conventions (e.g. repo seems to use a lot of 15_000 vs 15000)

I has reset the parameter of timeout to 5000, and used timeout function to set pipe testcase separately.