-
Notifications
You must be signed in to change notification settings - Fork 0
feat: apply code changes #16
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,30 +2,41 @@ | |
| # See: https://circleci.com/docs/configuration-reference | ||
| version: 2.1 | ||
|
|
||
| # Define a job to be invoked later in a workflow. | ||
| # See: https://circleci.com/docs/jobs-steps/#jobs-overview & https://circleci.com/docs/configuration-reference/#jobs | ||
| orbs: | ||
| node: circleci/node@6 | ||
|
|
||
| jobs: | ||
| say-hello: | ||
| # Specify the execution environment. You can specify an image from Docker Hub or use one of our convenience images from CircleCI's Developer Hub. | ||
| # See: https://circleci.com/docs/executor-intro/ & https://circleci.com/docs/configuration-reference/#executor-job | ||
| validate-config: | ||
| docker: | ||
| # Specify the version you desire here | ||
| # See: https://circleci.com/developer/images/image/cimg/base | ||
| - image: cimg/base:current | ||
| - image: cimg/base:stable | ||
| steps: | ||
| - checkout | ||
| - run: | ||
| name: Validate CircleCI configuration syntax | ||
| command: | | ||
| curl -fLSs https://raw.githubusercontent.com/CircleCI-Public/circleci-cli/main/install.sh | bash -s -- --no-sudo --install-dir /tmp/circleci-cli | ||
| /tmp/circleci-cli/circleci config validate .circleci/config.yml | ||
|
|
||
| # Add steps to the job | ||
| # See: https://circleci.com/docs/jobs-steps/#steps-overview & https://circleci.com/docs/configuration-reference/#steps | ||
| build: | ||
| docker: | ||
| - image: cimg/node:22.19 | ||
|
LCSOGthb marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 HIGH RISK Referencing pipeline images by mutable tags like '22.19' can lead to security risks and non-deterministic builds. Using an immutable SHA256 hash ensures that the exact same image is used every time. |
||
| steps: | ||
| # Checkout the code as the first step. | ||
| - checkout | ||
| - run: | ||
| name: "Say hello" | ||
| command: "echo Hello, World!" | ||
| name: Install dependencies | ||
| command: npm ci --legacy-peer-deps | ||
|
LCSOGthb marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM RISK The use of '--legacy-peer-deps' ignores peer dependency conflicts, which can lead to an unstable dependency tree and runtime issues. While it unblocks the immediate deploy, it is safer to resolve the underlying version mismatches in aq-dashboard/package.json. Try running the following prompt in your coding agent: > Resolve peer dependency conflicts in aq-dashboard/package.json so that the --legacy-peer-deps flag can be removed from the build steps. |
||
| working_directory: aq-dashboard | ||
| - run: | ||
| name: Build | ||
| command: npm run build | ||
| working_directory: aq-dashboard | ||
| - run: | ||
| name: Test | ||
| command: npm test -- --watchAll=false --passWithNoTests | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM RISK Suggestion: Avoid using '--passWithNoTests' in CI. This flag can lead to false positives where the build passes despite no tests being executed due to misconfiguration. |
||
| working_directory: aq-dashboard | ||
|
|
||
| # Orchestrate jobs using workflows | ||
| # See: https://circleci.com/docs/workflows/ & https://circleci.com/docs/configuration-reference/#workflows | ||
| workflows: | ||
| say-hello-workflow: # This is the name of the workflow, feel free to change it to better match your workflow. | ||
| # Inside the workflow, you define the jobs you want to run. | ||
| build-workflow: | ||
| jobs: | ||
| - say-hello | ||
| - validate-config | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚪ LOW RISK Suggestion: The validate-config job is redundant because CircleCI natively validates the configuration before pipeline execution. Additionally, it currently runs in parallel with the build job, failing to act as a gateway. Try running the following prompt in your coding agent: > Remove the validate-config job and the node orb from .circleci/config.yml, and update the workflow to only include the build job. |
||
| - build | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import React from 'react'; | ||
| import { render, screen } from '@testing-library/react'; | ||
| import App from './App'; | ||
|
|
||
| // Stub geolocation so Dashboard does not throw in jsdom | ||
| beforeEach(() => { | ||
| Object.defineProperty(global.navigator, 'geolocation', { | ||
|
Check warning on line 7 in aq-dashboard/src/App.test.tsx
|
||
| configurable: true, | ||
| value: { | ||
| getCurrentPosition: jest.fn(), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 MEDIUM RISK Suggestion: The getCurrentPosition mock is a bare jest.fn(), which does not call its success or error callbacks. This prevents testing the component behavior after a location is resolved or when an error occurs. Try running the following prompt in your coding agent: > Refactor the geolocation mock in App.test.tsx to support simulating both successful location responses and errors in different test cases. |
||
| }, | ||
| }); | ||
| }); | ||
|
|
||
| // Stub leaflet, which relies on a browser DOM unavailable in jsdom | ||
| jest.mock('react-leaflet', () => ({ | ||
| MapContainer: ({ children }: { children: React.ReactNode }) => ( | ||
| <div data-testid="map-container">{children}</div> | ||
| ), | ||
| TileLayer: () => <div data-testid="tile-layer" />, | ||
| })); | ||
|
|
||
| jest.mock('axios'); | ||
|
|
||
| describe('App', () => { | ||
| it('renders without crashing', () => { | ||
| render(<App />); | ||
| }); | ||
|
|
||
| it('shows the waiting-for-location prompt while geolocation is pending', () => { | ||
| render(<App />); | ||
| expect(screen.getByText(/waiting for location/i)).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('shows a button to enter location manually', () => { | ||
| render(<App />); | ||
| expect( | ||
| screen.getByRole('button', { name: /enter location manually/i }) | ||
| ).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 HIGH RISK
The 'stable' tag is inherently volatile. Pin this image to a specific SHA256 hash to prevent breaking changes in your CI pipeline.