fix(parser): respect escape for timestamp syntax#39457
Open
IsseiHasegawa wants to merge 8 commits intoRocketChat:developfrom
Open
fix(parser): respect escape for timestamp syntax#39457IsseiHasegawa wants to merge 8 commits intoRocketChat:developfrom
IsseiHasegawa wants to merge 8 commits intoRocketChat:developfrom
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds grammar support for escaped timestamp syntax and integrates it into inline and strikethrough parsing; expands test coverage with fuzz/property-based tests, a strike-format Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
No issues found across 1 file
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/message-parser/tests/fuzz.test.ts (3)
43-49: This property is redundant with the next one.
parse(input)throwing already fails the determinism property on Lines 52-56, so this adds runtime without adding much signal. I’d either drop it or replace it with a stronger structural invariant on the returned AST.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/message-parser/tests/fuzz.test.ts` around lines 43 - 49, The test named "parses valid parser-like inputs without throwing" is redundant with the determinism property that already fails if parse(input) throws; remove that test or replace it with a stronger structural invariant: instead of only asserting no throw, call parse(input) and assert the returned AST meets expected shape (for example, non-null object, has expected root type, and required fields like nodes/children or type tags). Locate the test that uses parserInputArbitrary and parse and either delete that test block or change its body to validate the parsed AST structure rather than merely checking for exceptions.
23-23: Cover both strike delimiters here.The grammar accepts both
~...~and~~...~~, but this arbitrary only hits the single-tilde path. A parser bug in the double-tilde branch would slip through this fuzz suite.Proposed direction
-const strikeArbitrary = fc.oneof(timestampArbitrary, relativeTimestampArbitrary, plainTextArbitrary).map((value) => `~${value}~`); +const strikeArbitrary = fc + .tuple( + fc.oneof(timestampArbitrary, relativeTimestampArbitrary, plainTextArbitrary), + fc.constantFrom('~', '~~'), + ) + .map(([value, delimiter]) => `${delimiter}${value}${delimiter}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/message-parser/tests/fuzz.test.ts` at line 23, The strikeArbitrary currently only generates single-tilde strikes (`~...~`) so add generation for double-tilde strikes as well: update strikeArbitrary (which composes timestampArbitrary, relativeTimestampArbitrary, plainTextArbitrary) to randomly choose between wrapping the inner value with "~" and with "~~" (e.g., use an extra fc.oneof or fc.constant that yields the delimiter and then map to `${delim}${value}${delim}`), ensuring both `~...~` and `~~...~~` branches are covered.
17-19: Tighten the mention generator to match the grammar more closely.This currently misses legal user-mention forms such as repeated
@prefixes andname:name/name@name, while still allowing punctuation-only cases like@-or@.that are not valid mentions. That makes the"valid parser-like inputs"label a bit misleading and leaves real mention branches under-fuzzed.Proposed direction
-const mentionArbitrary = fc - .stringMatching(/[a-z0-9._-]{1,12}/) - .map((value) => `@${value}`); +const mentionNameArbitrary = fc.oneof( + fc.stringMatching(/[A-Za-z0-9][A-Za-z0-9._-]{0,11}/), + fc + .tuple( + fc.stringMatching(/[A-Za-z0-9][A-Za-z0-9._-]{0,5}/), + fc.constantFrom(':', '@'), + fc.stringMatching(/[A-Za-z0-9][A-Za-z0-9._-]{0,5}/), + ) + .map(([left, separator, right]) => `${left}${separator}${right}`), +); + +const mentionArbitrary = fc + .tuple(fc.integer({ min: 1, max: 3 }), mentionNameArbitrary) + .map(([prefixCount, value]) => `${'@'.repeat(prefixCount)}${value}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/message-parser/tests/fuzz.test.ts` around lines 17 - 19, The mentionArbitrary generator is too permissive and misses valid forms (repeated leading @ and segment separators) while allowing punctuation-only names; update the stringMatching regex used by mentionArbitrary to require one or more '@' prefixes followed by one or more alphanumeric segments separated only by valid separators (':' or '@' or '.' or '-') where each segment must contain at least one alphanumeric character and the whole mention stays within the intended length (e.g., 1–12 chars after prefix); keep the .map((v) => v) wrapper (or existing map) but replace the current /[a-z0-9._-]{1,12}/ pattern with a stricter pattern that enforces these segment rules so mentionArbitrary produces real-world mentions like @@alice, alice:bob, and name@name while rejecting punctuation-only cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/message-parser/tests/fuzz.test.ts`:
- Around line 43-49: The test named "parses valid parser-like inputs without
throwing" is redundant with the determinism property that already fails if
parse(input) throws; remove that test or replace it with a stronger structural
invariant: instead of only asserting no throw, call parse(input) and assert the
returned AST meets expected shape (for example, non-null object, has expected
root type, and required fields like nodes/children or type tags). Locate the
test that uses parserInputArbitrary and parse and either delete that test block
or change its body to validate the parsed AST structure rather than merely
checking for exceptions.
- Line 23: The strikeArbitrary currently only generates single-tilde strikes
(`~...~`) so add generation for double-tilde strikes as well: update
strikeArbitrary (which composes timestampArbitrary, relativeTimestampArbitrary,
plainTextArbitrary) to randomly choose between wrapping the inner value with "~"
and with "~~" (e.g., use an extra fc.oneof or fc.constant that yields the
delimiter and then map to `${delim}${value}${delim}`), ensuring both `~...~` and
`~~...~~` branches are covered.
- Around line 17-19: The mentionArbitrary generator is too permissive and misses
valid forms (repeated leading @ and segment separators) while allowing
punctuation-only names; update the stringMatching regex used by mentionArbitrary
to require one or more '@' prefixes followed by one or more alphanumeric
segments separated only by valid separators (':' or '@' or '.' or '-') where
each segment must contain at least one alphanumeric character and the whole
mention stays within the intended length (e.g., 1–12 chars after prefix); keep
the .map((v) => v) wrapper (or existing map) but replace the current
/[a-z0-9._-]{1,12}/ pattern with a stricter pattern that enforces these segment
rules so mentionArbitrary produces real-world mentions like @@alice, alice:bob,
and name@name while rejecting punctuation-only cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dbca6a66-b988-48f1-8147-f475bdb2d391
📒 Files selected for processing (1)
packages/message-parser/tests/fuzz.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/message-parser/tests/fuzz.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:14.766Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx:63-70
Timestamp: 2026-03-06T18:02:11.732Z
Learning: In RocketChat/Rocket.Chat, tests in the `packages/gazzodown` package (and likely the broader test suite) are always expected to run in the UTC timezone. This makes `toLocaleString()` output deterministic, so snapshot tests that include locale/timezone-sensitive content (such as `title` attributes from `toLocaleString()`) are stable and do not need to be replaced with targeted assertions.
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
packages/message-parser/tests/fuzz.test.ts
📚 Learning: 2026-03-06T18:09:14.766Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:14.766Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Applied to files:
packages/message-parser/tests/fuzz.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
packages/message-parser/tests/fuzz.test.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
packages/message-parser/tests/fuzz.test.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
packages/message-parser/tests/fuzz.test.ts
Author
|
I added an additional property-based fuzz test using fast-check. The generator creates parser-like inputs (timestamps, mentions, bold, strike, inline code, and plain text) and verifies:
This helps provide additional safety coverage for future parser changes. |
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/message-parser/src/grammar.pegjs">
<violation number="1" location="packages/message-parser/src/grammar.pegjs:267">
P2: Escaped timestamp rules normalize the date via timestampFromIsoTime/timestampFromHours, so escaped literals no longer round‑trip to the original text (e.g., ISO input becomes Unix seconds). Escaped output should preserve the raw matched text.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/message-parser/src/grammar.pegjs`:
- Around line 266-276: The StrikethroughContent grammar currently references
TimestampRules directly which allows escaped timestamps to bypass strike
parsing; update the StrikethroughContent rule to include EscapedTimestampRules
in the same places where TimestampRules are used (mirror how InlineItemPattern
uses EscapedTimestampRules/TimestampRules) so that sequences like
~~\<t:1708551317:R>~~ are parsed as an escaped timestamp inside a strike; ensure
you replace or augment any direct TimestampRules references in
StrikethroughContent with EscapedTimestampRules (and keep TimestampRules
fallback as needed) to preserve existing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 295c8506-2acf-4c95-9599-4c8b606d7afe
📒 Files selected for processing (2)
packages/message-parser/src/grammar.pegjspackages/message-parser/tests/escaped.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/message-parser/tests/escaped.test.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx:63-70
Timestamp: 2026-03-06T18:02:20.381Z
Learning: In RocketChat/Rocket.Chat, tests in the `packages/gazzodown` package (and likely the broader test suite) are always expected to run in the UTC timezone. This makes `toLocaleString()` output deterministic, so snapshot tests that include locale/timezone-sensitive content (such as `title` attributes from `toLocaleString()`) are stable and do not need to be replaced with targeted assertions.
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
packages/message-parser/tests/escaped.test.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
packages/message-parser/tests/escaped.test.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
packages/message-parser/tests/escaped.test.ts
🔇 Additional comments (1)
packages/message-parser/tests/escaped.test.ts (1)
18-18: Nice regression case.This keeps the assertion narrow and directly exercises the new escaped-timestamp inline path.
Author
|
I found and fixed the escaped timestamp parsing issue. Previously, an input like I also re-ran the relevant timestamp tests and the full message-parser test suite locally. |
IsseiHasegawa
changed the title
Author
|
I updated EscapedTimestampRules to preserve the raw matched timestamp text instead of reconstructing it from normalized values. I also added EscapedTimestampRules before TimestampRules in StrikethroughContent so escaped timestamps are handled correctly inside strike formatting. I re-ran the timestamp tests locally and they passed. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/message-parser/src/grammar.pegjs (1)
266-276: Extract the shared timestamp payload grammar.
EscapedTimestampRulesnow mirrors the same date alternatives and optional-format shape asTimestampRules. It works, but the next timestamp syntax change will need two grammar edits again. Please factor the common payload into a dedicated rule so the escaped and unescaped paths stay in sync.♻️ Proposed refactor
+TimestampValue = Unixtime / ISO8601Date / ISO8601DateWithoutMilliseconds / Timestamp + -TimestampRules = "<t:" date:(Unixtime / ISO8601Date / ISO8601DateWithoutMilliseconds / Timestamp) ":" format:TimestampType ">" { return timestamp(date, format); } / "<t:" date:(Unixtime / ISO8601Date / ISO8601DateWithoutMilliseconds / Timestamp) ">" { return timestamp(date); } +TimestampRules = "<t:" date:TimestampValue ":" format:TimestampType ">" { return timestamp(date, format); } / "<t:" date:TimestampValue ">" { return timestamp(date); } EscapedTimestampRules - = "\\" "<t:" rawDate:$(Unixtime / ISO8601Date / ISO8601DateWithoutMilliseconds / Timestamp) ":" format:TimestampType ">" { + = "\\" "<t:" rawDate:$(TimestampValue) ":" format:TimestampType ">" { return plain(`<t:${rawDate}:${format}>`); } - / "\\" "<t:" rawDate:$(Unixtime / ISO8601Date / ISO8601DateWithoutMilliseconds / Timestamp) ">" { + / "\\" "<t:" rawDate:$(TimestampValue) ">" { return plain(`<t:${rawDate}>`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/message-parser/src/grammar.pegjs` around lines 266 - 276, Create a shared payload rule (e.g., TimestampPayload or TimestampInner) that encapsulates the rawDate alternatives (Unixtime / ISO8601Date / ISO8601DateWithoutMilliseconds / Timestamp) and the optional ":" format:TimestampType, then replace the duplicated rawDate/format parts in both EscapedTimestampRules and TimestampRules to call that new rule so both branches produce the same plain(`<t:${rawDate}>`) / plain(`<t:${rawDate}:${format}>`) outputs; update InlineItemPattern to continue referencing EscapedTimestampRules and TimestampRules unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/message-parser/src/grammar.pegjs`:
- Around line 266-276: Create a shared payload rule (e.g., TimestampPayload or
TimestampInner) that encapsulates the rawDate alternatives (Unixtime /
ISO8601Date / ISO8601DateWithoutMilliseconds / Timestamp) and the optional ":"
format:TimestampType, then replace the duplicated rawDate/format parts in both
EscapedTimestampRules and TimestampRules to call that new rule so both branches
produce the same plain(`<t:${rawDate}>`) / plain(`<t:${rawDate}:${format}>`)
outputs; update InlineItemPattern to continue referencing EscapedTimestampRules
and TimestampRules unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb8f87e0-546e-4ce4-92e8-aa0ebf51d576
📒 Files selected for processing (1)
packages/message-parser/src/grammar.pegjs
📜 Review details
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx:63-70
Timestamp: 2026-03-06T18:02:20.381Z
Learning: In RocketChat/Rocket.Chat, tests in the `packages/gazzodown` package (and likely the broader test suite) are always expected to run in the UTC timezone. This makes `toLocaleString()` output deterministic, so snapshot tests that include locale/timezone-sensitive content (such as `title` attributes from `toLocaleString()`) are stable and do not need to be replaced with targeted assertions.
📚 Learning: 2026-03-06T18:09:17.867Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/DateTimeFormats.spec.tsx:20-23
Timestamp: 2026-03-06T18:09:17.867Z
Learning: In the RocketChat/Rocket.Chat gazzodown package (`packages/gazzodown`), tests are intended to run under the UTC timezone, but as of PR `#39397` this is NOT yet explicitly enforced in `jest.config.ts` or the `package.json` test scripts (which just run `jest` without `TZ=UTC`). To make timezone-sensitive snapshot tests reliable across all environments, `TZ=UTC` should be added to the test scripts in `package.json` or to `jest.config.ts` via `testEnvironmentOptions.timezone`. Without explicit UTC enforcement, snapshot tests involving date-fns formatted output or `toLocaleString()` will fail for contributors in non-UTC timezones.
Applied to files:
packages/message-parser/src/grammar.pegjs
📚 Learning: 2026-03-06T18:02:20.381Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/elements/Timestamp/RelativeTime.spec.tsx:63-70
Timestamp: 2026-03-06T18:02:20.381Z
Learning: In RocketChat/Rocket.Chat, tests in the `packages/gazzodown` package (and likely the broader test suite) are always expected to run in the UTC timezone. This makes `toLocaleString()` output deterministic, so snapshot tests that include locale/timezone-sensitive content (such as `title` attributes from `toLocaleString()`) are stable and do not need to be replaced with targeted assertions.
Applied to files:
packages/message-parser/src/grammar.pegjs
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
packages/message-parser/src/grammar.pegjs
🔇 Additional comments (1)
packages/message-parser/src/grammar.pegjs (1)
580-582: Nice catch on the strike-specific parser path.Mirroring the escaped-before-unescaped ordering here closes the branch that previously bypassed the new inline fix.
Member
|
Hi @IsseiHasegawa , we're migrating away from the Alsatian framework for Apps-Engine tests here #37592 So we won't be accepting files that add new Alsatian tests |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a test case to ensure that timestamps with the relative format (
R) are correctly parsed when used inside strike formatting.This extends the existing timestamp test coverage for strike elements.
Testing
Proposed changes (including videos or screenshots)
Issue(s)
COMM-144
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Tests
Chores