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

Upgrade to Vue 3 #1306

Closed
kinow opened this issue May 15, 2022 · 20 comments · Fixed by #1480
Closed

Upgrade to Vue 3 #1306

kinow opened this issue May 15, 2022 · 20 comments · Fixed by #1480
Assignees

Comments

@kinow
Copy link
Member

kinow commented May 15, 2022

Creating an issue so I don't lose track of this.

We are using Bootstrap Vue for the UI. It gives developers the same that vanilla Bootstrap offers, but instead of using CSS classes directly, it gives them Vue components, ready to use.

While it was great for rapid development for the migration of Backbone.JS to Vue.js, unfortunately now we are stuck with Vue 2 as Bootstrap Vue has not upgraded to Vue 3 yet - bootstrap-vue/bootstrap-vue#5196

I think we might be able to move to vanilla Bootstrap and create simple components, assuming we are not using too many Bootstrap Vue components.

Alternatively, we can move to another UI library, with the disadvantage that unless it's a variation of Bootstrap, we would have a different UI look and feel.

Once this is solved, we might be ready to move to Vue 3 and Vite.

@kinow kinow self-assigned this May 15, 2022
@kinow
Copy link
Member Author

kinow commented May 15, 2022

List of components we are using:

kinow@ranma:~/Development/java/jena/jena/jena-fuseki2/jena-fuseki-ui$ grep -r -H -h -E "<b-.*[\s|>]" -o src | awk -F" |>" '{ print $1 }' | cut -c2- | sort | uniq -c | sort -h -r
     32 b-col
     26 b-row
     13 b-container
      9 b-card-body
      7 b-card-header
      7 b-card
      5 b-alert
      4 b-spinner
      3 b-skeleton
      3 b-nav-item
      2 b-navbar-nav
      2 b-nav
      2 b-lis
      2 b-input-group
      2 b-form-s
      2 b-card-title
      1 b-tr
      1 b-th
      1 b-progress
      1 b-overlay
      1 b-navbar-toggle
      1 b-navbar-brand
      1 b-navbar
      1 b-list-group
      1 b-input-group-append
      1 b-form
      1 b-collapse
      1 b-button
      1 b-btn
      1 b-badge

Most components are really easy to substitute, like b-form (a simple <div> in Bootstrap 5), b-tr, b-navbar, b-col, b-row, etc. Will take a while to get it all done and tested, but doesn't look too complicated (boring? yes 😄 but we gotta do what we must do).

@afs
Copy link
Member

afs commented May 15, 2022

😄 just for getting the regex to work!

@afs
Copy link
Member

afs commented May 20, 2022

Same title as #1251. Is this a duplicate? continuation? sub-task?

@kinow
Copy link
Member Author

kinow commented May 22, 2022

Bug in the regex, some b-form-* elements were swallowed by the regex. Here's the current list:

kinow@ranma:~/Development/java/jena/jena/jena-fuseki2/jena-fuseki-ui$ grep -r -h -E "<b\-[^ >]+" -o src | awk -F" " '{ print $1 }' | cut -c2- | sort | uniq -c | sort -h -r
     11 b-form-group
     10 b-nav-item
      9 b-card-body
      7 b-card-header
      7 b-card
      5 b-form-input
      4 b-table
      4 b-spinner
      3 b-skeleton
      3 b-popover
      2 b-navbar-nav
      2 b-nav
      2 b-list-group-item
      2 b-form-select
      2 b-form
      2 b-card-title
      1 b-textarea
      1 b-progress
      1 b-pagination
      1 b-overlay
      1 b-navbar-toggle
      1 b-navbar-brand
      1 b-navbar
      1 b-list-group
      1 b-form-radio-group
      1 b-collapse

@afs afs added the Fuseki UI label Jun 2, 2022
afs added a commit that referenced this issue Aug 14, 2022
GH-1306, GH-1443, JENA-2307: Replace Bootstrap Vue by vanilla Bootstrap 5 CSS classes
@afs
Copy link
Member

afs commented Aug 14, 2022

Major progress - PR "Replace Bootstrap Vue by vanilla Bootstrap 5 CSS classes" #1307 merged.

kinow added a commit to kinow/jena that referenced this issue Aug 14, 2022
kinow added a commit to kinow/jena that referenced this issue Aug 14, 2022
kinow added a commit to kinow/jena that referenced this issue Aug 14, 2022
kinow added a commit to kinow/jena that referenced this issue Aug 14, 2022
kinow added a commit to kinow/jena that referenced this issue Sep 19, 2022
kinow added a commit to kinow/jena that referenced this issue Sep 19, 2022
kinow added a commit to kinow/jena that referenced this issue Sep 21, 2022
kinow added a commit to kinow/jena that referenced this issue Sep 21, 2022
kinow added a commit to kinow/jena that referenced this issue Sep 21, 2022
kinow added a commit to kinow/jena that referenced this issue Sep 21, 2022
kinow added a commit to kinow/jena that referenced this issue Oct 31, 2022
kinow added a commit to kinow/jena that referenced this issue Oct 31, 2022
kinow added a commit to kinow/jena that referenced this issue Nov 1, 2022
kinow added a commit that referenced this issue Nov 1, 2022
kinow added a commit that referenced this issue Nov 1, 2022
@afs
Copy link
Member

afs commented Nov 5, 2022

Hi @kinow,

I'm having problems in the maven build with jena-fuseki-ui.
But it works on Jenkins!

I'm assuming it is a configuration problem on my side.

I did a fairly clean Ubuntu install on this machine but local setting for my user account were copied over from the old machine.

Steps

A clean git-clone of Jena.

cd jena-fuseki-ui
## chrome, clear the cache.

## Sometimes succeeds
mvn clean verify

## Always fails after a success.
mvn clean verify

Test failure

AssertionError: Timed out retrying after 7500ms: Expected to find element: ``table.jena-table > thead > tr > th``, but never found it.

screenshot_datasets

Setup

cypress is installed (npm install cypress) globally.

npm install cypress --save-dev in jena-fuseki-ui didn't work.

It is different to before - there is only one cypress failure in datasets.

Attached information

Attached: Full log of mvn clean verify
Attached: screen shot screenshot_datasets
Attached: failed cypress "save-dev" install

Extract of the maven output:

[INFO] [TESTS]   8 passing (15s)
[INFO] [TESTS]   1 failing
[INFO] [TESTS] 
[INFO] [TESTS]   1) datasets
[INFO] [TESTS]        After creating new datasets
[INFO] [TESTS]          Edits the graph:
[INFO] [TESTS]      AssertionError: Timed out retrying after 7500ms: Expected to find element: `table.jena-table > thead > tr > th`, but never found it.
[INFO] [TESTS]       at Context.eval (webpack://apache-jena-fuseki/./tests/e2e/specs/datasets.cy.js:291:9)

[INFO] [TESTS]   (Screenshots)
[INFO] [TESTS] 
[INFO] [TESTS]   -  tests/e2e/screenshots/datasets.cy.js/datasets -- After creating new datasets --      (1280x720)
[INFO] [TESTS]      Edits the graph (failed).png                                                                   

[INFO] [TESTS]  ERROR  Error: Command failed: /home/afs/ASF/afs-jena/jena-fuseki2/jena-fuseki-ui/node_modules/cypress/bin/cypress run --config baseUrl=http://localhost:8080 --headless
[INFO] [TESTS] Error: Command failed: /home/afs/ASF/afs-jena/jena-fuseki2/jena-fuseki-ui/node_modules/cypress/bin/cypress run --config baseUrl=http://localhost:8080 --headless
[INFO] [TESTS]     at makeError (/home/afs/ASF/afs-jena/jena-fuseki2/jena-fuseki-ui/node_modules/execa/index.js:174:9)
[INFO] [TESTS]     at /home/afs/ASF/afs-jena/jena-fuseki2/jena-fuseki-ui/node_modules/execa/index.js:278:16
[INFO] [TESTS]     at processTicksAndRejections (node:internal/process/task_queues:96:5)
[INFO] [TESTS] vue-cli-service test:e2e --headless exited with code 1
[INFO] --> Sending SIGTERM to other processes..
[INFO] [SERVER] yarn run serve:fuseki exited with code SIGTERM
[INFO] error Command failed with exit code 1.
[INFO] info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  44.360 s
[INFO] Finished at: 2022-11-05T09:33:42Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal com.github.eirslett:frontend-maven-plugin:1.12.1:yarn (yarn run test:e2e) on project jena-fuseki-ui: Failed to run task: 'yarn run test:e2e -- --headless --browser chrome' failed. org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1) -> [Help 1]

@kinow
Copy link
Member Author

kinow commented Nov 5, 2022

Huh interesting. Maybe I didn't write the test well enough and it could be improved. Will have a look at this issue this weekend Andy.

@afs
Copy link
Member

afs commented Nov 5, 2022

The only workaround I can find is to disable the e2e tests with <phase>none</phase>.

  1. This seems to be a build/test issue and not related to the produced pages.
  2. It runs sometimes, and runs on a completely clean setup (Jenkins).
  3. It works for you.

So it seems the webapp is functioning correctly.

Will have a look at this issue this weekend

Have a weekend!

We could check-in pom.xml with a disabled setup as a short term solution and there isn't a rush.

@kinow
Copy link
Member Author

kinow commented Nov 5, 2022

So it looks like this could be really just a brittle test.

Have a weekend

I am! Just passed by sagrada familia, now drinking a clara with boquerones. But I have a delivery at work for next Friday, so i will be online in a few hours anyway.

I will try to make the test more reliable, but if I cannot make it, I will mark it as optional so that that doesn't fail the build (and add a note to fix it).

Lots of users are moving from Cypress to Playwright. But I think I will refrain from another major change in the UI after Vue2 -> Vue3 (and we had the Backbonejs -> Vue2 before too).

https://www.reddit.com/r/javascript/comments/yd3dr8/on_migrating_from_cypress_to_playwright/

@kinow
Copy link
Member Author

kinow commented Nov 5, 2022

Executed tests four times with mvn clean verify and got all runs to pass with no errors.

@kinow
Copy link
Member Author

kinow commented Nov 5, 2022

@afs can you try this patch and let me know if the output of the test changes, please?

diff --git a/jena-fuseki2/jena-fuseki-ui/tests/e2e/specs/datasets.cy.js b/jena-fuseki2/jena-fuseki-ui/tests/e2e/specs/datasets.cy.js
index 35f5e98595..303ce2cd60 100644
--- a/jena-fuseki2/jena-fuseki-ui/tests/e2e/specs/datasets.cy.js
+++ b/jena-fuseki2/jena-fuseki-ui/tests/e2e/specs/datasets.cy.js
@@ -287,6 +287,9 @@ describe('datasets', () => {
         .contains('Loading')
         .should('not.exist')
       // Now the table must have the new columns with the graph name and count.
+      cy
+        .get('table.jena-table > thead > tr > th')
+        .should('be.visible')
       cy
         .get('table.jena-table > thead > tr > th')
         .eq(0)

@afs
Copy link
Member

afs commented Nov 5, 2022

Tried on two machines - my works laptop and my personal desktop.

The problems I encountered are my personal desktop
The laptop is fine - ran twice with code as it is in git, no problems.

Desktop: with the change the screen shot show error at "12" as before which is presumably the inserted test.

I have no idea what the difference between the machines could possibly be! I'll search. At least I can run a build and a release on some setup.

@kinow
Copy link
Member Author

kinow commented Nov 6, 2022

Interesting. The part about clearing the cache shouldn't be a requirement for Cypress. It launches the browser you select to run the e2e tests (or the default browser), and uses a separate profile and also customizes its initialization (e.g. which extension to use, what browser settings to have, etc) 1.

Footnotes

  1. https://docs.cypress.io/guides/guides/launching-browsers#Browser-Environment

@afs
Copy link
Member

afs commented Nov 6, 2022

I have a sort-of fix -- #1606.

The two machines I have tried are very similar software-wise but the working on (laptop) runs Wayland for graphics and the other (desktop, with an NVIDIA GeForce card) run X11. nvida+wayland isn't perfect and glitches a bit.

desktop+wayland works most of the time.

The test fail at a 7.5 second timeout (not that it seems to be that long).

So what's odd about the Fuseki UI? Guess: loading Triply which is a slow point.
Increasing the cypress defaultCommandTimeout to 20s and I can do a mvn clean verify on the desktop with X11 as graphics.

(I tried different default system browser but no joy there.)

@kinow
Copy link
Member Author

kinow commented Nov 6, 2022

At another project I think I had a default timeout of 60 seconds 😄 so that looks like a good fix to me. Thanks for troubleshooting it @afs !

afs added a commit that referenced this issue Nov 6, 2022
@afs afs mentioned this issue Nov 7, 2022
@afs
Copy link
Member

afs commented Nov 7, 2022

Not good I'm afraid. It's still going wrong in about the same place though the screenshot is slightly different (can see more of the previous tests).

datasets -- After creating new datasets -- Edits the graph (failed)

I took out all the 'table.jena-table > tbody > tr > td' tests and it then fails at "wait @getGraph"
datasets -- After creating new datasets -- Edits the graph (failed)

Setting a much longer timeout does not help - but does confirm it is the defaultCommandTimeout that is happening. But i may be that the wanted content is never going to appear.

If I move the block of tests to directly after the "after", it seems to be more reliable. But after yesterday, I'm not that confident it is stable.

    after(() => {
      // Special endpoint that clears the datasets data.
      cy.request('/tests/reset')
    })
    it('Edits the graph', () => {
...

This is PR#1607

But other than that I have run out of ideas.

@kinow
Copy link
Member Author

kinow commented Nov 7, 2022

I will try to reproduce it again. If I manage to get an environment with the same error I can troubleshoot it.

Basically what I will use for that is the chrome browser, either using Cypress' tools (like the step by step execution, that one that you move your mouse over the test steps to see what the UI looks like) with the browser console, or removing parts of the test until I can isolate what's causing it.

When I do the latter I always change the test name so Cypress runs just that one test, and not everything. You can try that locally by renaming any test to blabla.only Adding .only tells Cypress to run just that one test. It is one of the limitations in Cypress that I believe others have fixed in Playwright and other tools.

@afs
Copy link
Member

afs commented Nov 8, 2022

The problem is my machine so don't worry too much.

If it interferes with doing a release, I can either use another machine, or switch out the test just for the release.

@phillipross
Copy link
Contributor

The only workaround I can find is to disable the e2e tests with <phase>none</phase>.

  1. This seems to be a build/test issue and not related to the produced pages.
  2. It runs sometimes, and runs on a completely clean setup (Jenkins).
  3. It works for you.

So it seems the webapp is functioning correctly.

Will have a look at this issue this weekend

Have a weekend!

We could check-in pom.xml with a disabled setup as a short term solution and there isn't a rush.

Might it make sense to control enabling/disabling e2e via a maven profile? maybe define a new maven profile intended to be used by CI that would be easy to enable/disable by specifying an env var, system property, etc when maven is invoked?

@afs
Copy link
Member

afs commented Nov 13, 2022

Hi @phillipross,
Yes, though it seems to be only one machine, and it's become stable just be reordering. Which is a bit weird.
What makes it painful is that the machine is one of the places to do a release from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants