Skip to content

Commit

Permalink
fix[devtools/ci]: fixed incorrect condition calculation for @reactVer…
Browse files Browse the repository at this point in the history
…sion annotation (#26997)

Suppose that you have this setup for devtools test:
```
// @reactVersion <= 18.1
// @reactVersion >= 17.1
```

With previous implementation, the accumulated condition will be `"<=
18.1" && ">= 17.1"`, which is just `">= 17.1"`, when evaluated. That's
why we executed some tests for old versions of react on main (and
failed).

With these changes the resulting condition will be `"<= 18.1 >= 17.1"`,
not using `&&`, because semver does not support this operator. All
currently failing tests will be skipped now as expected.

Also increased timeout value for shell server to start
  • Loading branch information
hoxyq authored Jun 23, 2023
1 parent 70e998a commit 8ec962d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import path from 'path';
import {rimrafSync} from 'rimraf';

describe('x_google_ignoreList source map extension', () => {
jest.setTimeout(30 * 1000);
jest.setTimeout(60 * 1000);

const pathToExtensionsPackage = path.resolve(__dirname, '..', '..');
const pathToChromeExtensionBuild = path.join(
Expand Down
40 changes: 19 additions & 21 deletions scripts/babel/transform-react-version-pragma.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,31 +26,29 @@ function transform(babel) {
return null;
}

let conditions = null;
for (const line of comments) {
const commentStr = line.value.trim();
if (commentStr.startsWith(GATE_VERSION_STR)) {
const condition = t.stringLiteral(
commentStr.slice(GATE_VERSION_STR.length)
);
if (conditions === null) {
conditions = [condition];
} else {
conditions.push(condition);
const resultingCondition = comments.reduce(
(accumulatedCondition, commentLine) => {
const commentStr = commentLine.value.trim();

if (!commentStr.startsWith(GATE_VERSION_STR)) {
return accumulatedCondition;
}

const condition = commentStr.slice(GATE_VERSION_STR.length);
if (accumulatedCondition === null) {
return condition;
}
}
}

if (conditions !== null) {
let condition = conditions[0];
for (let i = 1; i < conditions.length; i++) {
const right = conditions[i];
condition = t.logicalExpression('&&', condition, right);
}
return condition;
} else {
return accumulatedCondition.concat(' ', condition);
},
null
);

if (resultingCondition === null) {
return null;
}

return t.stringLiteral(resultingCondition);
}

return {
Expand Down
2 changes: 1 addition & 1 deletion scripts/circleci/run_devtools_e2e_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function runTestShell() {
// Assume the test shell server failed to start.
logError('Testing shell server failed to start');
exitWithCode(1);
}, 30000);
}, 60 * 1000);

This comment has been minimized.

Copy link
@othmanjabes

othmanjabes Jun 24, 2023

The previous choice is better
Because it reduces the number of running processes


logBright('Starting testing shell server');

Expand Down

0 comments on commit 8ec962d

Please sign in to comment.