Skip to content

Conversation

@jjomier
Copy link
Contributor

@jjomier jjomier commented Jan 13, 2026

This new feature allows to select multiple builds in a group and move them as a batch or mark the builds expected.

  • Added fix when image for project is not set
  • Modified icons for better look and feel
image

@williamjallen williamjallen self-requested a review January 14, 2026 02:17
Copy link
Collaborator

@williamjallen williamjallen left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution. I left a few initial comments inline. Please also add before and after screenshots to the PR description for all UI changes.

@jjomier
Copy link
Contributor Author

jjomier commented Jan 15, 2026

Thanks @williamjallen I cleaned up the contribution and added a screenshot.

@williamjallen
Copy link
Collaborator

@jjomier Please see the failing CI. Please also add a test for the new functionality you've added. tests/cypress/e2e/expected-build.cy.js is probably the most relevant place to put new test cases.

@williamjallen
Copy link
Collaborator

@jjomier There's nothing wrong with debugging via the CI, but it may be more efficient to iterate locally. The instructions for running the test suite in Docker are here. Nothing other than Docker needs to be installed locally to run the tests.

@jjomier
Copy link
Contributor Author

jjomier commented Jan 18, 2026

Tests are very unstable on my side when running locally (works one time and fails the second time).
Is there any documentation to run the tests locally from a development branch - I'm probably missing something?

Forgot to mention I'm running in Docker.

@williamjallen
Copy link
Collaborator

@jjomier Which tests are failing for you locally? I run the tests every day in Docker and haven't noticed any flakiness recently, but it's possible that differences in our machines are revealing a flaky test somewhere. Some of the Cypress tests are flaky in CI. Feel free to open issues for any unstable tests and I can dig a little deeper.

For development, you can use the cdash_copy_source alias in the container to copy files from the host for most iterations. If you're building the frontend, you'll need to run npm run dev afterwards. The cdash_install --dev command is an all-in-one which copies data and runs through all of the installation scripts, but takes a little bit longer.

@jjomier
Copy link
Contributor Author

jjomier commented Jan 18, 2026

Thanks for the input! Maybe we can document these commands for other contributors.

@jjomier
Copy link
Contributor Author

jjomier commented Jan 19, 2026

Thanks for the patience and help @williamjallen - seems like unit tests are passing now. Feel free to review and modify as needed. This feature will help quite a bit when moving batch builds to new build group.

Copy link
Collaborator

@williamjallen williamjallen left a comment

Choose a reason for hiding this comment

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

A few UI comments:

  1. Let's remove the blue color from the info icon so it doesn't stick out as much.
  2. Let's reverse the order of these buttons so logging in simply adds a button rather than changing the layout of both:
    Image
  3. Clicking this button briefly shows the "Mark as Not Done" button until the animation finishes. The same applies to the "Mark as Expected" button.
    Image
  4. Clicking this button leads to a painfully slow animation as the modal appears:
    Image
  5. It looks like the inner table now has new column highlighting. Let's remove that.
    Image

@jjomier
Copy link
Contributor Author

jjomier commented Jan 20, 2026

@williamjallen what is the best way to reinitialize the testing database. Seems like tests are failing locally and not sure how to reset the database to a proper state.

@williamjallen
Copy link
Collaborator

@jjomier The install_1 test drops and recreates the database, so there shouldn't be anything else necessary. Depending on which test is failing and how it failed, you may also need to delete the contents of /cdash/storage/app/**/*. You can also check the log file to make sure it's identical to .env.dev.

@jjomier
Copy link
Contributor Author

jjomier commented Jan 20, 2026

I would need some help fixing this test as I cannot get the tests to run correctly: now getting over 20 tests failing - wondering if switching to master branch caused any isses.

Is there a way to start fresh? (I tried to remove containers without any luck).

@williamjallen
Copy link
Collaborator

@jjomier If you switched to master, you probably have more migrations locally than you should. You can delete them with rm /cdash/database/migrations/*. To start fresh, you should delete all containers and volumes, and then start everything back up again. I suspect you removed the containers but not the volumes so the database persisted.

@jjomier
Copy link
Contributor Author

jjomier commented Jan 21, 2026

@williamjallen thanks for the help. Should be all good. Removing the transition for the modal caused the old test to fail, which was doing a accept then cancel (not sure why). Should be all good now.

Copy link
Collaborator

@williamjallen williamjallen left a comment

Choose a reason for hiding this comment

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

One last code comment to resolve and then this should be good to go.

Comment on lines +115 to +122
if (rowCount > 1) {
cy.get('#project_5_13').find('tbody').find('tr').eq(1).find('[data-cy="build-admin-options"]').click();
cy.get('#project_5_13').find('tbody').find('tr').eq(1).find('table.animate-show').should('be.visible');
cy.get('[data-cy="mark-as-expected-btn"]').should('exist');
cy.get('[data-cy="mark-as-non-expected-btn"]').should('not.exist');
// close admin options
cy.get('#project_5_13').find('tbody').find('tr').eq(1).find('[data-cy="build-admin-options"]').click();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove this conditional so there's only one code path the test can take.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants