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

create user test and researchplan extended test are added #1379

Merged
merged 8 commits into from
Jul 14, 2023

Conversation

mehmood86
Copy link
Contributor

  • rather 1-story 1-commit than sub-atomic commits

  • commit title is meaningful => git history search

  • commit description is helpful => helps the reviewer to understand the changes

  • code is up-to-date with the latest developments of the target branch (rebased to it or whatever) => ⏩-merge for linear history is favoured

  • added code is linted

  • tests are passing (at least locally): we still have some random test failure on CI. thinking of asking spec/examples.txt to be commited

  • in case the changes are visible to the end-user,  video or screenshots should be added to the PR => helps with user testing

  • testing coverage improvement is improved.

  • CHANGELOG :  add a bullet point on top (optional: reference to github issue/PR )

  • parallele PR for documentation  on docusaurus  if the feature/fix is tagged for a release

@github-actions
Copy link

LCOV of commit 5da9995 during Continuous Integration #1193

Summary coverage rate:
  lines......: 60.9% (12481 of 20486 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

Copy link
Collaborator

@JanCBrammer JanCBrammer left a comment

Choose a reason for hiding this comment

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

Minor formatting issue: the indentation level is inconsistent across the files. E.g., admin.cy.js is indented using 4 spaces, whereas it's 2 spaces in research_plan_with_user.cy.js. I suggest formatting according to ESLint.

spec/cypress/end_to_end/research_plan_with_user.cy.js Outdated Show resolved Hide resolved
spec/cypress/end_to_end/admin.cy.js Show resolved Hide resolved
spec/cypress/end_to_end/research_plan_with_user.cy.js Outdated Show resolved Hide resolved
@@ -13,4 +13,36 @@ describe('Research Plan with User', () => {
cy.contains('My Research Plan 2');
cy.get('#tabList-tab-4 > span').contains('1(0)');
});

it('rename research plan', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I ran this tests locally with yarn cypress run --project spec --spec "spec/cypress/end_to_end/research_plan_with_user.cy.js", and it fails with

1) Research Plan with User
       rename research plan:
     AssertionError: Timed out retrying after 5000ms: Expected to find element: `[width="280px"] > :nth-child(1) > .preview-table`, but never found it.
      at Context.eval (webpack:///./cypress/end_to_end/research_plan_with_user.cy.js:9:7)

  2) Research Plan with User
       rename research plan:
     AssertionError: Timed out retrying after 5000ms: Expected to find element: `[width="280px"] > :nth-child(1) > .preview-table`, but never found it.
      at Context.eval (webpack:///./cypress/end_to_end/research_plan_with_user.cy.js:24:7)

Not sure if it fails because of a timeout on my machine or because the selector ('[width="280px"] > :nth-child(1) > .preview-table') is too brittle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dont know, it runs fine on my end. But anyway I added cypress identifier for this part and I hope now it should work seamlessly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I increased the timeout from 5000 to 10000 and tried running on chrome instead of electron, Still won't work 🤔

1) Research Plan with User
       rename research plan:
     AssertionError: Timed out retrying after 10000ms: Expected to find element: `[data-cy="researchPLanItem-1"]`, but never found it.
      at Context.eval (webpack:///./cypress/end_to_end/research_plan_with_user.cy.js:9:7)

  2) Research Plan with User
       add/remove fields in research plan:
     AssertionError: Timed out retrying after 10000ms: Expected to find element: `[data-cy="researchPLanItem-1"]`, but never found it.
      at Context.eval (webpack:///./cypress/end_to_end/research_plan_with_user.cy.js:24:7)

Copy link
Collaborator

Choose a reason for hiding this comment

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

The test now passed on my machine.

@github-actions
Copy link

github-actions bot commented Jul 3, 2023

LCOV of commit 6c01461 during Continuous Integration #1224

Summary coverage rate:
  lines......: 59.4% (12272 of 20658 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@JanCBrammer JanCBrammer mentioned this pull request Jul 14, 2023
10 tasks
@github-actions
Copy link

LCOV of commit 405f57f during Continuous Integration #1298

Summary coverage rate:
  lines......: 59.8% (12386 of 20705 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

Copy link
Collaborator

@JanCBrammer JanCBrammer left a comment

Choose a reason for hiding this comment

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

Looks good to me.
We removed the collection API tests for now since they (at least conceptually) replicate
https://github.com/ComPlat/chemotion_ELN/blob/0190e0fcdc35d62d84107e465a7941030bdee259/spec/api/collection_api_spec.rb.

@JanCBrammer JanCBrammer merged commit 0367488 into main Jul 14, 2023
1 check passed
@github-actions
Copy link

LCOV of commit e6b8119 during Continuous Integration #1299

Summary coverage rate:
  lines......: 59.8% (12386 of 20705 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

mehreenmansur pushed a commit that referenced this pull request Jul 14, 2023
* create user test  and researchplan extended test are added

* added identifier to eliminate flakyness in researcPlan cypress-specs

* collections api spec is now added

* deleted unnecessary comments from collection_api_spec.cy.js

* Remove redundant API test for now (conceptually replicated RSpec)

---------

Co-authored-by: Jan C. Brammer <jan.c.brammer@gmail.com>
mehreenmansur pushed a commit that referenced this pull request Jul 31, 2023
* create user test  and researchplan extended test are added

* added identifier to eliminate flakyness in researcPlan cypress-specs

* collections api spec is now added

* deleted unnecessary comments from collection_api_spec.cy.js

* Remove redundant API test for now (conceptually replicated RSpec)

---------

Co-authored-by: Jan C. Brammer <jan.c.brammer@gmail.com>
@JanCBrammer JanCBrammer deleted the end_to_end_tests_ext branch August 16, 2023 11:21
mekkyz pushed a commit that referenced this pull request Sep 21, 2023
* create user test  and researchplan extended test are added

* added identifier to eliminate flakyness in researcPlan cypress-specs

* collections api spec is now added

* deleted unnecessary comments from collection_api_spec.cy.js

* Remove redundant API test for now (conceptually replicated RSpec)

---------

Co-authored-by: Jan C. Brammer <jan.c.brammer@gmail.com>
baolanlequang pushed a commit that referenced this pull request Mar 5, 2024
* create user test  and researchplan extended test are added

* added identifier to eliminate flakyness in researcPlan cypress-specs

* collections api spec is now added

* deleted unnecessary comments from collection_api_spec.cy.js

* Remove redundant API test for now (conceptually replicated RSpec)

---------

Co-authored-by: Jan C. Brammer <jan.c.brammer@gmail.com>
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.

None yet

2 participants