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

test: add chips test case #19

Merged
merged 3 commits into from
Jun 2, 2024
Merged

test: add chips test case #19

merged 3 commits into from
Jun 2, 2024

Conversation

fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Dec 26, 2023

eggjs/egg-cookies#42

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

Summary by CodeRabbit

  • New Features

    • Introduced CI workflow for Node.js projects.
    • Added GitHub Actions workflow for releasing Node.js applications.
  • Documentation

    • Updated README: Removed build status, dependencies, and vulnerabilities badges. Improved installation command and added syntax highlighting for code snippets.
  • Chores

    • Added package-lock.json to .gitignore.
    • Updated devDependencies and scripts in package.json.
  • Tests

    • Added new test case for "chips" functionality.
    • Added new files and configurations for session handling in test fixtures.

.expect({ foo: 'bar' })
.expect(res => {
const cookie = res.headers['set-cookie'].join('|');
assert(cookie.includes('; secure; httponly; partitioned'));
Copy link
Member Author

@fengmk2 fengmk2 Dec 26, 2023

Choose a reason for hiding this comment

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

EGG_SESS=W3tTMcKNjtfn5iLXgbKqGHMg9Nd4jQeyMT4HG0wVkxjtuYL7_D9GWO5ofA3i18-rKvrIu9eOdLgOgXraaETTjQSmlBhwvEulvUS6h1ZKNVc=; path=/; max-age=86400; expires=Wed, 27 Dec 2023 14:31:08 GMT; secure; httponly; partitioned

Copy link

coderabbitai bot commented Jun 2, 2024

Walkthrough

The recent updates introduce new CI workflows for Node.js projects, enhance the .gitignore file, and update the README.md by removing badges and tweaking installation instructions. The package.json file sees version updates and script adjustments, while new test cases and configurations are added for session handling in the chips module.

Changes

Files/Paths Change Summary
.github/workflows/node.yml Introduced a CI workflow for Node.js projects.
.github/workflows/release.yml Added a workflow for releasing Node.js applications.
.gitignore Added package-lock.json to ignored files.
README.md Removed various badges and updated installation command and syntax highlighting.
package.json Updated devDependencies versions and modified scripts for linting and testing.
test/app/middleware/session.test.js Added a new test case for "chips" functionality.
test/fixtures/chips/.../home.js Introduced methods for retrieving and updating session data.
test/fixtures/chips/.../router.js Defined new routes for handling GET requests.
test/fixtures/chips/.../config.default.js Added configurations for keys, session handling, and proxy settings.
test/fixtures/chips/package.json Introduced a JSON configuration file for the chips-session package.

Poem

In the land of code, a change did brew,
CI workflows fresh, and badges bid adieu.
Sessions now with chips, so crisp and new,
Dependencies updated, scripts refined too.
With each commit, our project grew,
A rabbit's touch, in every view.
🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Copy link

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert Package NoteSource
Install scripts npm/[email protected]
  • Install script: postinstall
  • Source: node scripts/postinstall.js

View full report↗︎

Next steps

What is an install script?

Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts.

Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

Copy link

codecov bot commented Jun 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.65%. Comparing base (374f6e2) to head (79fd5db).

Additional details and impacted files
@@             Coverage Diff              @@
##            master      #19       +/-   ##
============================================
- Coverage   100.00%   89.65%   -10.35%     
============================================
  Files            4        4               
  Lines           28       87       +59     
  Branches         0       14       +14     
============================================
+ Hits            28       78       +50     
- Misses           0        9        +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Outside diff range and nitpick comments (3)
test/app/middleware/session.test.js (3)

Line range hint 1-1: Remove redundant 'use strict' directive.

JavaScript modules are strict by default, and the 'use strict' directive is redundant here.


Line range hint 5-5: Update import statement to use the 'node:' protocol.

- const sleep = require('mz-modules/sleep');
+ const sleep = require('node:mz-modules/sleep');

This change ensures that the import statement explicitly states that it is importing a Node.js built-in module, aligning with modern Node.js practices.


Line range hint 233-353: Consider using 'for...of' instead of 'forEach' for better readability and performance.

- ['cookie-session', 'memory-session', 'memory-session-generator', 'redis-session'].forEach(name => {
+ for (const name of ['cookie-session', 'memory-session', 'memory-session-generator', 'redis-session']) {

This change enhances readability and aligns with modern JavaScript practices by using for...of instead of forEach.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 374f6e2 and 79fd5db.

Files selected for processing (10)
  • .github/workflows/node.yml (1 hunks)
  • .github/workflows/release.yml (1 hunks)
  • .gitignore (1 hunks)
  • README.md (3 hunks)
  • package.json (2 hunks)
  • test/app/middleware/session.test.js (1 hunks)
  • test/fixtures/chips/app/controller/home.js (1 hunks)
  • test/fixtures/chips/app/router.js (1 hunks)
  • test/fixtures/chips/config/config.default.js (1 hunks)
  • test/fixtures/chips/package.json (1 hunks)
Files skipped from review due to trivial changes (3)
  • .github/workflows/release.yml
  • .gitignore
  • test/fixtures/chips/package.json
Additional context used
Biome
test/fixtures/chips/app/router.js

[error] 1-1: Redundant use strict directive.

test/fixtures/chips/config/config.default.js

[error] 1-1: Redundant use strict directive.

test/fixtures/chips/app/controller/home.js

[error] 1-1: Redundant use strict directive.

test/app/middleware/session.test.js

[error] 1-1: Redundant use strict directive.


[error] 5-5: A Node.js builtin module should be imported with the node: protocol.


[error] 233-353: Prefer for...of instead of forEach.

LanguageTool
README.md

[uncategorized] ~33-~33: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...lt ``` ### External Store egg-session support external store, you can store your sess...


[uncategorized] ~101-~101: You might be missing the article “an” here.
Context: ...) {} // }; }; ``` Once you use external session store, session is strong depend...

Additional comments not posted (8)
test/fixtures/chips/app/router.js (1)

3-6: Ensure that the controller methods home.get and home.set are correctly implemented to handle these routes.

Verification successful

The implementations of home.get and home.set methods are correctly found in the following files:

  • test/fixtures/chips/app/controller/home.js
  • test/fixtures/memory-session/app/controller/home.js

Both methods are implemented as follows:

  • home.get: Sets ctx.body to ctx.session.
  • home.set: Sets ctx.session to ctx.query and then sets ctx.body to ctx.session.

These implementations appear to be correct and handle the routes as expected.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the implementation of `home.get` and `home.set`.

# Test: Search for the implementation of `home.get` and `home.set`.
ast-grep --lang javascript --pattern $'exports.get = async ctx => { $$$ }'
ast-grep --lang javascript --pattern $'exports.set = async ctx => { $$$ }'

Length of output: 1160

test/fixtures/chips/config/config.default.js (1)

3-9: Configuration settings for keys, session, and proxy are correctly defined. Ensure these settings align with the application's requirements.

test/fixtures/chips/app/controller/home.js (1)

3-10: The controller methods get and set are implemented correctly. Ensure they are properly integrated with the rest of the application.

Verification successful

The integration of the home.get and home.set methods has been verified successfully. They are used in multiple router files across different test fixtures, indicating proper integration with the application.

  • home.get is used in:

    • test/fixtures/chips/app/router.js
    • test/fixtures/session-maxage-session/app/router.js
    • test/fixtures/samesite-none-session/app/router.js
    • test/fixtures/memory-session/app/router.js
    • test/fixtures/redis-session/app/router.js
    • test/fixtures/memory-session-generator/app/router.js
    • test/fixtures/httponly-false-session/app/router.js
    • test/fixtures/cookie-session/app/router.js
    • test/fixtures/logValue-false-session/app/router.js
  • home.set is used in:

    • test/fixtures/chips/app/router.js
    • test/fixtures/samesite-none-session/app/router.js
    • test/fixtures/redis-session/app/router.js
    • test/fixtures/memory-session/app/router.js
    • test/fixtures/session-maxage-session/app/router.js
    • test/fixtures/logValue-false-session/app/router.js
    • test/fixtures/memory-session-generator/app/router.js
    • test/fixtures/httponly-false-session/app/router.js
    • test/fixtures/cookie-session/app/router.js
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the integration of `home.get` and `home.set` with the application.

# Test: Search for the usage of `home.get` and `home.set`.
rg --type javascript $'home.get'
rg --type javascript $'home.set'

Length of output: 133



Script:

#!/bin/bash
# Description: Verify the integration of `home.get` and `home.set` with the application.

# Test: Search for the usage of `home.get` and `home.set`.
rg 'home.get'
rg 'home.set'

Length of output: 2014

.github/workflows/node.yml (1)

1-17: The CI workflow configuration looks good. Ensure the custom action node-modules/github-actions/.github/workflows/node-test-mysql.yml@master is up-to-date and maintained.

Verification successful

The previous attempt to verify the custom action failed due to an incorrect JSON field. Let's correct this by using the appropriate fields to gather information about the workflows in the repository.


The repository node-modules/github-actions has its default branch as master and no latest release. To ensure the custom action node-test-mysql.yml exists and is up-to-date, we need to list the contents of the .github/workflows directory in the repository.


The custom action node-test-mysql.yml exists in the repository node-modules/github-actions and is located in the .github/workflows directory. This confirms that the action referenced in the CI workflow is present and can be used.

Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the custom action used in the workflow.

# Test: Check the repository for the latest version of the action.
gh repo view node-modules/github-actions --json workflows

Length of output: 1160



Script:

#!/bin/bash
# Description: Verify the custom action used in the workflow.

# Test: Check the repository for the latest version of the action.
gh repo view node-modules/github-actions --json name,defaultBranchRef,latestRelease

Length of output: 169



Script:

#!/bin/bash
# Description: List the contents of the .github/workflows directory in the repository.

# Test: Verify the existence of the node-test-mysql.yml file.
gh repo clone node-modules/github-actions temp-repo
ls temp-repo/.github/workflows

Length of output: 356

package.json (1)

Line range hint 28-45: The updates to dependencies and scripts in package.json are appropriate. Ensure the new scripts test-local and cov are functioning as expected.

README.md (2)

19-19: The updated installation command in the README is correct and follows npm best practices.


109-109: The new configuration code snippet for logValue is clear and well-documented. Ensure it aligns with the actual implementation in the code.

Verification successful

The logValue configuration is indeed present in the codebase and is used in various files, including configuration and test files. This confirms that the new configuration code snippet for logValue in the README aligns with the actual implementation in the code.

  • config/config.default.js: Defines logValue with both true and false values.
  • test/app/middleware/session.test.js: Contains multiple tests related to logValue.
  • app.js: Uses logValue to conditionally log session values.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the `logValue` configuration in the actual code.

# Test: Search for the `logValue` configuration in the codebase.
rg --type javascript $'logValue'

Length of output: 66



Script:

#!/bin/bash
# Description: Verify the `logValue` configuration in the actual code.

# Test: Search for the `logValue` configuration in the codebase.
rg --type js 'logValue'

Length of output: 1160

test/app/middleware/session.test.js (1)

95-117: Review the new test suite for "chips".

The new test suite for "chips" is well-structured and follows the existing patterns in the file. It properly initializes the app, sets up the agent, and includes clean-up steps. The test itself is asynchronous and checks the session behavior correctly.

@@ -0,0 +1,6 @@
'use strict';
Copy link

Choose a reason for hiding this comment

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

Remove redundant 'use strict' directive.

- 'use strict';
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
'use strict';
Tools
Biome

[error] 1-1: Redundant use strict directive.

@@ -0,0 +1,9 @@
'use strict';
Copy link

Choose a reason for hiding this comment

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

Remove redundant 'use strict' directive.

- 'use strict';
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
'use strict';
Tools
Biome

[error] 1-1: Redundant use strict directive.

@@ -0,0 +1,10 @@
'use strict';
Copy link

Choose a reason for hiding this comment

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

Remove redundant 'use strict' directive.

- 'use strict';
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
'use strict';
Tools
Biome

[error] 1-1: Redundant use strict directive.

@fengmk2 fengmk2 merged commit 0f06e00 into master Jun 2, 2024
9 of 11 checks passed
@fengmk2 fengmk2 deleted the chips-test branch June 2, 2024 14:35
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.

1 participant