Skip to content

Conversation

tanyatrayanava
Copy link
Contributor

No description provided.

},
"devDependencies": {
"@types/chance": "1.1.3",
"@types/edit-json-file": "^1.7.0",

Choose a reason for hiding this comment

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

Always better to pin the version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 46 to 48
await t.expect(timestampAfterUpdate).notEql(timestampForEdit);
await t.expect(cloudTitle).notEql(cloudValueForEdit);
await t.expect(cloudDescription).notEql(cloudValueForEdit);

Choose a reason for hiding this comment

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

Please add error message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Comment on lines 38 to 48
const buildFilePathNew = editJsonFile(buildPath);
const createRedisFilePathNew = editJsonFile(createRedisPath);
//Check the promo button
await t.expect(myRedisDatabasePage.promoButton.textContent).notContains(cloudValueForEdit, 'Promo button text is updated');
//Check the json files are automatically updated
const timestampAfterUpdate = await buildFilePathNew.get('timestamp');
const cloudTitle = await createRedisFilePathNew.get('cloud.title');
const cloudDescription = await createRedisFilePathNew.get('cloud.description');
await t.expect(timestampAfterUpdate).notEql(timestampForEdit);
await t.expect(cloudTitle).notEql(cloudValueForEdit);
await t.expect(cloudDescription).notEql(cloudValueForEdit);

Choose a reason for hiding this comment

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

Something here isn't clear enough.
Do the first two lines are apart of the test or pre requisite?
It's hard to understand from reading the code the test case, and I feel that in case the test will be fail, shouldn't be easy understand the bug (and if the bug is on the product or test).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these first two lines are necessary because the data that is in these files is cached and needs to be overwritten to get the updated information
I also added more comments to this test

ofersteinberg
ofersteinberg previously approved these changes Apr 18, 2022
@tanyatrayanava tanyatrayanava merged commit e61f5e3 into main Apr 20, 2022
@tanyatrayanava tanyatrayanava deleted the feature/e2e-automatically-update branch April 20, 2022 11:23
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.

2 participants