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

Fix xod/core/select description and add test patch #1598

Merged
merged 1 commit into from
Dec 26, 2018

Conversation

brusherru
Copy link
Contributor

@brusherru brusherru commented Dec 24, 2018

But we have a problem to write such tests in the same library because tabtests will get an error:

CONFLICTING_SPECIALIZATIONS_FOR_ABSTRACT_PATCH {"patchPath":"xod/core/if-else","conflictingSpecializations":["xod/core/if-else(number)","@/if-else(number)"],"trace":["tabtest/local/test-select-4-numbers","@/test-select-4-numbers…

So probably we have to extract such tests into separate xod/test-core? 🤔
Also, in this case, will be no problems with position of defers in the program graph :)

@nkrkv
Copy link
Member

nkrkv commented Dec 25, 2018

I can’t suggest anything constructive straight away, but I’d like to note that the problem is deeper than the choice of where the test should reside. This is related to #888 as well. The problem is when we have a library opened as a project. At this time we effectively have the contents loaded twice. A poper solution should address the library development experience.

But until we have no solution, a separate project test-core sounds reasonable. But, please, do not pollute userspace with it. Create it under ./workspace/test-core, not under ./workspace/__lib__/xod

@brusherru brusherru force-pushed the fix-xod-core-select-description branch from 2314c43 to 929db9e Compare December 25, 2018 17:26
@brusherru brusherru requested review from nkrkv and removed request for nkrkv December 25, 2018 17:33
Copy link
Member

@nkrkv nkrkv left a comment

Choose a reason for hiding this comment

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

They should be under CI, but they are not currently:

  test-cpp:
    docker: *docker-custom-nodejs
    environment:
      XOD_OUTPUT: /tmp/tabtests-cpp
    steps:
      - checkout
      - restore_cache: *restore-node_modules
      - run: *step-install
      - run:
          name: Build CLI
          command: yarn build:cli
      - run:
          name: Generate tabtests
          command: | # --------------------- here
            ls -d workspace/__lib__/xod/* | xargs -n 1 \
              yarn xodc tabtest --no-build | cat
      - run:
          name: Build tabtests
          command: make -C $XOD_OUTPUT
      - run:
          name: Run tabtests
          command: make -C $XOD_OUTPUT test

Should be:

ls -d workspace/__lib__/xod/* workspace/test-core | xargs ...

And if so, would it better to put the test under a common subdirectory (workspace/test?) to avoid touching the config in future?!

@brusherru brusherru force-pushed the fix-xod-core-select-description branch from 929db9e to c568c3c Compare December 26, 2018 10:25
Copy link
Contributor

@evgenykochetkov evgenykochetkov left a comment

Choose a reason for hiding this comment

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

👍🏻

@brusherru brusherru requested a review from nkrkv December 26, 2018 10:38
workspace/test/core/project.xod Outdated Show resolved Hide resolved
@brusherru brusherru force-pushed the fix-xod-core-select-description branch from c568c3c to c5816b1 Compare December 26, 2018 11:05
@brusherru brusherru merged commit 6f9e1d5 into master Dec 26, 2018
@brusherru brusherru deleted the fix-xod-core-select-description branch December 26, 2018 11:26
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

3 participants